Git Merge Request #316: Covert BeautifulSoup to BeautifulSoup4 (rejected)

Merging...

Merged

Something went wrong. Please, merge manually

Checking if merge is possible...

Something went wrong. Please, merge manually

Abhishek Chaudhary wants to merge 1 commit from /u/abhishek/abhi-allura/ to master, 2020-04-10

Covert BeautifulSoup to BeautifulSoup4.

Commit Date  
[7083cf] (master) by Abhishekism9450 Abhishekism9450

Change BeautifulSoup to BeautifulSoup4

2019-03-08 16:58:40 Tree

Discussion

  • Dave Brondsema

    Dave Brondsema - 2019-03-08

    Hi, this is looking good. It will need some updates though:

    Remove Allura/allura/lib/macro.py.bak and the root/if-you-need-to-delete-this-open-an-issue-async-disk-cache files. We don't want those added to the repo.

    When I run the test suite, I get some errors. The output from bs4 must be a little bit different than before. So those tests will have to be updated to pass again.

    And if I run pip uninstall BeautifulSoup to remove the old version, and then run the test suite, I get quite a lot of errors :( These are from tests that use r.html which is using bs4 behind the scenes when BeautifulSoup is not there. So those will all have to be updated too.

    As you're going through the code, please try to stick with PEP8 code style guidelines. For example: assert str(expected) == str(rendered_tree) is better than assert str(expected)==str(rendered_tree) and html, 'html5lib' is better than html,'html5lib'. It doesn't have to be perfect (there is plenty of existing code that isn't), but nice to be consistent when we add or change code. Your editor probably can help with highlighting formatting PEP8 issues.

     
  • Abhishek Chaudhary

    Hello Dave,
    I removed the following files from repo, As you mentioned about uninstalling the BeautifulSoup, I tried the same but it is showing "It is not Installed" .

    About the Test, the error which I am facing is :

    File "/allura-data/virtualenv/local/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(self.arg)
    File "/allura/Allura/allura/tests/decorators.py", line 82, in wrapped
    return func(
    args, **kw)
    File "/allura/Allura/allura/tests/functional/test_trovecategory.py", line 125, in test_trove_hierarchy
    assert str(expected) == str(rendered_tree)

    AssertionError:
    -------------------- >> begin captured stdout << ---------------------
    Running setup_app() from allura.websetup

    --------------------- >> end captured stdout << ----------------------

    Can you help me in this, so i can keep working on upgrading new dependencies.

    P.S. I didn't commit any new changes yet waiting to get rid of failures and then push the modify repo.

     
    • Dave Brondsema

      Dave Brondsema - 2019-03-11

      Hi,

      If it says its BeautifulSoup is not installed, maybe your python virtual enviroment isn't active? You should run pip the same way you run the test suite. You can check with echo $VIRTUAL_ENV to see if it shows anything. If not, run . env-allura/bin/activate again.

      For the test error, you will need to print out the expected and actual results, and see what is different. If it is just a minor syntax difference, then update the test's expected value to match. By the way, you can run one test at a time like nosetests Allura/allura/tests/functional/test_trovecategory.py:TestTroveCategoryController.test_trove_hierarchy which can help when working on a single test at a time.

       
  • Abhishek Chaudhary

    Hi Dave,
    I upgraded 2 more dependancy i.e Cryptography and decorator. Please have a look.
    By the way, I'm using docker to run the test suites. In my terminal it does not show any test failure but some are Skipped. If you can give me any snapshot of any failure, then i can actually work on modifying that code.

     
  • Dave Brondsema

    Dave Brondsema - 2019-03-13

    Hi,

    Can you submit a separate merge request for the different upgrades? I don't see them in this merge request and it'd be good to keep it separate from beautiful soup issues anyway. So a separate branch without the beautifulsoup changes would be necessary, and then make a merge request from that.

    Running the test suites in docker is fine, and some skipped tests are ok too. Just make sure that you get the new versions installed within docker via docker-compose run web pip install -r requirements.txt

    Thanks!

     
  • Dave Brondsema

    Dave Brondsema - 2020-04-10
    • Status: open --> rejected
     

Log in to post a comment.