• 
      

    Add new unit tests for RBTools's Git support

    Review Request #8099 — Created April 4, 2016 and submitted

    Information

    RBTools
    master

    Reviewers

    Add new unit tests for RBTools's Git support (this review request covers most of the tests in the project description at https://student-sonar.herokuapp.com/projects)

    Wrote the following test cases for the following methods of the the GitClient class -

    Method name - push_upstream() -
    1) Tested push_upstream() with an invalid target branch. The 'git_pull' must raise a PushError exception.
    2) Tested push_upstream() with the push_url set to be an invalid one. Neither the 'pull' or the 'push' must raise an exception because it gets it's origin_url from the Git config which still has a valid fetch_url

    Method name - merge()
    1) Tested merge() with invalid target and destination branches. Both of these must raise a MergeError exception
    2) Tested merge() with squash set to True and False. Used a KGB spy to check if execute was called with the right arguments.

    Method name - create_commit()
    1) Tested create_commit() with run_editor set to True and False. Used a KGB spy to check if edit_text was called or not.
    2) Tested create_commit() with all_files set to True and False. Used a KGB spy to check if execute was called with the right arguments.

    Method name - delete_branch()
    1) Tested delete_branch() with merged_only set to True and False. Used a KGB spy to check if execute was called with the right arguments.

    Description From Last Updated

    Col: 59 W291 trailing whitespace

    reviewbotreviewbot

    Col: 45 W291 trailing whitespace

    reviewbotreviewbot

    Col: 60 W291 trailing whitespace

    reviewbotreviewbot

    local variable 'e' is assigned to but never used

    reviewbotreviewbot

    Col: 45 W291 trailing whitespace

    reviewbotreviewbot

    Col: 1 W293 blank line contains whitespace

    reviewbotreviewbot

    Col: 5 E303 too many blank lines (2)

    reviewbotreviewbot

    Col: 63 W291 trailing whitespace

    reviewbotreviewbot

    Col: 63 W291 trailing whitespace

    reviewbotreviewbot

    Col: 45 W291 trailing whitespace

    reviewbotreviewbot

    Col: 58 W291 trailing whitespace

    reviewbotreviewbot

    Col: 63 W291 trailing whitespace

    reviewbotreviewbot

    Col: 77 W291 trailing whitespace

    reviewbotreviewbot

    Col: 45 W291 trailing whitespace

    reviewbotreviewbot

    Col: 64 W291 trailing whitespace

    reviewbotreviewbot

    Col: 76 W291 trailing whitespace

    reviewbotreviewbot

    Col: 32 E711 comparison to None should be 'if cond is not None:'

    reviewbotreviewbot

    Col: 65 W291 trailing whitespace

    reviewbotreviewbot

    Col: 75 W291 trailing whitespace

    reviewbotreviewbot

    Col: 68 W291 trailing whitespace

    reviewbotreviewbot

    Col: 32 E711 comparison to None should be 'if cond is not None:'

    reviewbotreviewbot

    Col: 45 W291 trailing whitespace

    reviewbotreviewbot

    Col: 60 E231 missing whitespace after ':'

    reviewbotreviewbot

    Col: 68 W291 trailing whitespace

    reviewbotreviewbot

    Col: 57 E231 missing whitespace after ':'

    reviewbotreviewbot

    Col: 60 E231 missing whitespace after ':'

    reviewbotreviewbot

    Col: 68 W291 trailing whitespace

    reviewbotreviewbot

    Col: 57 E231 missing whitespace after ':'

    reviewbotreviewbot

    Col: 60 E231 missing whitespace after ':'

    reviewbotreviewbot

    Col: 68 W291 trailing whitespace

    reviewbotreviewbot

    Col: 57 E231 missing whitespace after ':'

    reviewbotreviewbot

    Col: 33 E128 continuation line under-indented for visual indent

    reviewbotreviewbot

    Col: 60 E231 missing whitespace after ':'

    reviewbotreviewbot

    Col: 68 W291 trailing whitespace

    reviewbotreviewbot

    Col: 57 E231 missing whitespace after ':'

    reviewbotreviewbot

    Col: 33 E128 continuation line under-indented for visual indent

    reviewbotreviewbot

    You could put this after the push_upstream call on line 538 - that way, you wouldn't need the early return …

    mike_conleymike_conley

    Don't forget to test the success case too.

    mike_conleymike_conley

    assertTrue(False) after line 551? Same goes for other tests like this.

    mike_conleymike_conley

    Col: 33 E128 continuation line under-indented for visual indent

    reviewbotreviewbot

    Col: 33 E128 continuation line under-indented for visual indent

    reviewbotreviewbot

    I'm guessing this is just placeholder?

    mike_conleymike_conley

    Col: 50 E127 continuation line over-indented for visual indent

    reviewbotreviewbot

    Col: 50 E127 continuation line over-indented for visual indent

    reviewbotreviewbot

    Col: 50 E127 continuation line over-indented for visual indent

    reviewbotreviewbot

    Col: 50 E127 continuation line over-indented for visual indent

    reviewbotreviewbot

    Col: 33 E128 continuation line under-indented for visual indent

    reviewbotreviewbot

    Col: 33 E128 continuation line under-indented for visual indent

    reviewbotreviewbot

    Col: 76 W291 trailing whitespace

    reviewbotreviewbot

    """ on the next line.

    daviddavid

    This could all be done at once using six.assertRaisesRegexp.

    daviddavid

    Instead of catching the exception and doing assertTrue(False), just get rid of the try/except. An uncaught exception will cause the …

    daviddavid

    """ on the next line.

    daviddavid

    This can use six.assertRaisesRegexp

    daviddavid

    """ on the next line.

    daviddavid

    Can we reformat/change this to: author = type( b'Author', (object,), { 'fullname': 'name', 'email': 'email', })

    daviddavid

    six.assertRaisesMessage

    daviddavid

    """ on the next line.

    daviddavid

    If you add SpyAgency as a parent class, you can use self.spy_on(execute). This has several advantages: you don't have to …

    daviddavid

    Same formatting here as I mentioned above.

    daviddavid

    Col: 50 E127 continuation line over-indented for visual indent

    reviewbotreviewbot

    Col: 50 E127 continuation line over-indented for visual indent

    reviewbotreviewbot

    """ on the next line.

    daviddavid

    Col: 50 E127 continuation line over-indented for visual indent

    reviewbotreviewbot

    Col: 50 E127 continuation line over-indented for visual indent

    reviewbotreviewbot

    """ on the next line.

    daviddavid

    Same formatting here as above.

    daviddavid

    Col: 33 E128 continuation line under-indented for visual indent

    reviewbotreviewbot

    """ on the next line.

    daviddavid

    And here.

    daviddavid

    Col: 33 E128 continuation line under-indented for visual indent

    reviewbotreviewbot

    """ on the next line.

    daviddavid

    And here. Since we're using this in lots of places, perhaps we should define this once?

    daviddavid

    """ on the next line.

    daviddavid

    """ on the next line.

    daviddavid

    """ on the next line.

    daviddavid

    Col: 50 E127 continuation line over-indented for visual indent

    reviewbotreviewbot

    Col: 50 E127 continuation line over-indented for visual indent

    reviewbotreviewbot

    Col: 50 E127 continuation line over-indented for visual indent

    reviewbotreviewbot

    Col: 50 E127 continuation line over-indented for visual indent

    reviewbotreviewbot

    Col: 33 E128 continuation line under-indented for visual indent

    reviewbotreviewbot

    Col: 33 E128 continuation line under-indented for visual indent

    reviewbotreviewbot

    Col: 33 E128 continuation line under-indented for visual indent

    reviewbotreviewbot

    Col: 33 E128 continuation line under-indented for visual indent

    reviewbotreviewbot
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          rbtools/clients/tests.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          rbtools/clients/tests.py
      
      
    2. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 59
       W291 trailing whitespace
      
    3. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 45
       W291 trailing whitespace
      
    4. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 60
       W291 trailing whitespace
      
    5. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
       local variable 'e' is assigned to but never used
      
    6. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 45
       W291 trailing whitespace
      
    7. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 1
       W293 blank line contains whitespace
      
    8. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 5
       E303 too many blank lines (2)
      
    9. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 63
       W291 trailing whitespace
      
    10. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 63
       W291 trailing whitespace
      
    11. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 45
       W291 trailing whitespace
      
    12. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 58
       W291 trailing whitespace
      
    13. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 63
       W291 trailing whitespace
      
    14. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 77
       W291 trailing whitespace
      
    15. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 45
       W291 trailing whitespace
      
    16. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 64
       W291 trailing whitespace
      
    17. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 76
       W291 trailing whitespace
      
    18. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 32
       E711 comparison to None should be 'if cond is not None:'
      
    19. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 65
       W291 trailing whitespace
      
    20. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 75
       W291 trailing whitespace
      
    21. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 68
       W291 trailing whitespace
      
    22. rbtools/clients/tests.py (Diff revision 1)
       
       
      Show all issues
      Col: 32
       E711 comparison to None should be 'if cond is not None:'
      
    23. 
        
    SS
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          rbtools/clients/tests.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          rbtools/clients/tests.py
      
      
    2. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 45
       W291 trailing whitespace
      
    3. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 60
       E231 missing whitespace after ':'
      
    4. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 68
       W291 trailing whitespace
      
    5. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 57
       E231 missing whitespace after ':'
      
    6. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 60
       E231 missing whitespace after ':'
      
    7. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 68
       W291 trailing whitespace
      
    8. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 57
       E231 missing whitespace after ':'
      
    9. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 60
       E231 missing whitespace after ':'
      
    10. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 68
       W291 trailing whitespace
      
    11. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 57
       E231 missing whitespace after ':'
      
    12. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 33
       E128 continuation line under-indented for visual indent
      
    13. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 60
       E231 missing whitespace after ':'
      
    14. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 68
       W291 trailing whitespace
      
    15. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 57
       E231 missing whitespace after ':'
      
    16. rbtools/clients/tests.py (Diff revision 2)
       
       
      Show all issues
      Col: 33
       E128 continuation line under-indented for visual indent
      
    17. 
        
    SS
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          rbtools/clients/tests.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          rbtools/clients/tests.py
      
      
    2. rbtools/clients/tests.py (Diff revision 3)
       
       
      Show all issues
      Col: 33
       E128 continuation line under-indented for visual indent
      
    3. rbtools/clients/tests.py (Diff revision 3)
       
       
      Show all issues
      Col: 33
       E128 continuation line under-indented for visual indent
      
    4. 
        
    SS
    mike_conley
    1. Hey - I think these are on the right track. Great job!

      Don't forget to test the success cases too.

    2. rbtools/clients/tests.py (Diff revision 3)
       
       
       
      Show all issues

      You could put this after the push_upstream call on line 538 - that way, you wouldn't need the early return in 541.

    3. rbtools/clients/tests.py (Diff revision 3)
       
       
      Show all issues

      Don't forget to test the success case too.

    4. rbtools/clients/tests.py (Diff revision 3)
       
       
      Show all issues

      assertTrue(False) after line 551? Same goes for other tests like this.

    5. rbtools/clients/tests.py (Diff revision 3)
       
       
       
      Show all issues

      I'm guessing this is just placeholder?

      1. Umm no it's not really a placeholder. For my create_commit() tests, I needed to call a fake method instead of edit_text() when it was called. So this is that fake method which is called instead of edit_text() (which is needed because edit_text opens up a Vim window and waits for user input). Hence, you'll see the corresponding line agency.spy_on(edit_text, call_fake=self.return_new_message) in my code. Is there a better way of doing this without having to define a new method (maybe using Lambda functions)?

    6. 
        
    SS
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          rbtools/clients/tests.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          rbtools/clients/tests.py
      
      
    2. rbtools/clients/tests.py (Diff revision 4)
       
       
      Show all issues
      Col: 50
       E127 continuation line over-indented for visual indent
      
    3. rbtools/clients/tests.py (Diff revision 4)
       
       
      Show all issues
      Col: 50
       E127 continuation line over-indented for visual indent
      
    4. rbtools/clients/tests.py (Diff revision 4)
       
       
      Show all issues
      Col: 50
       E127 continuation line over-indented for visual indent
      
    5. rbtools/clients/tests.py (Diff revision 4)
       
       
      Show all issues
      Col: 50
       E127 continuation line over-indented for visual indent
      
    6. rbtools/clients/tests.py (Diff revision 4)
       
       
      Show all issues
      Col: 33
       E128 continuation line under-indented for visual indent
      
    7. rbtools/clients/tests.py (Diff revision 4)
       
       
      Show all issues
      Col: 33
       E128 continuation line under-indented for visual indent
      
    8. rbtools/clients/tests.py (Diff revision 4)
       
       
      Show all issues
      Col: 76
       W291 trailing whitespace
      
    9. 
        
    SS
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          rbtools/clients/tests.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          rbtools/clients/tests.py
      
      
    2. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues
      Col: 50
       E127 continuation line over-indented for visual indent
      
    3. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues
      Col: 50
       E127 continuation line over-indented for visual indent
      
    4. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues
      Col: 50
       E127 continuation line over-indented for visual indent
      
    5. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues
      Col: 50
       E127 continuation line over-indented for visual indent
      
    6. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues
      Col: 33
       E128 continuation line under-indented for visual indent
      
    7. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues
      Col: 33
       E128 continuation line under-indented for visual indent
      
    8. 
        
    david
    1. 
        
    2. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues

      """ on the next line.

    3. rbtools/clients/tests.py (Diff revision 5)
       
       
       
       
       
       
       
       
       
      Show all issues

      This could all be done at once using six.assertRaisesRegexp.

    4. rbtools/clients/tests.py (Diff revision 5)
       
       
       
       
       
       
       
      Show all issues

      Instead of catching the exception and doing assertTrue(False), just get rid of the try/except. An uncaught exception will cause the test case to fail.

    5. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues

      """ on the next line.

    6. rbtools/clients/tests.py (Diff revision 5)
       
       
       
       
       
       
       
       
       
       
      Show all issues

      This can use six.assertRaisesRegexp

    7. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues

      """ on the next line.

    8. rbtools/clients/tests.py (Diff revision 5)
       
       
       
      Show all issues

      Can we reformat/change this to:

      author = type(
          b'Author',
          (object,),
          {
              'fullname': 'name',
              'email': 'email',
          })
      
    9. rbtools/clients/tests.py (Diff revision 5)
       
       
       
       
      Show all issues

      six.assertRaisesMessage

    10. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues

      """ on the next line.

    11. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues

      If you add SpyAgency as a parent class, you can use self.spy_on(execute). This has several advantages: you don't have to create a new SpyAgency for each test, and it will automatically unspy after each test case (which is both less code and more robust if there are unexpected exceptions).

    12. rbtools/clients/tests.py (Diff revision 5)
       
       
       
      Show all issues

      Same formatting here as I mentioned above.

    13. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues

      """ on the next line.

    14. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues

      """ on the next line.

    15. rbtools/clients/tests.py (Diff revision 5)
       
       
       
      Show all issues

      Same formatting here as above.

    16. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues

      """ on the next line.

    17. rbtools/clients/tests.py (Diff revision 5)
       
       
       
      Show all issues

      And here.

    18. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues

      """ on the next line.

    19. rbtools/clients/tests.py (Diff revision 5)
       
       
       
      Show all issues

      And here. Since we're using this in lots of places, perhaps we should define this once?

    20. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues

      """ on the next line.

    21. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues

      """ on the next line.

    22. rbtools/clients/tests.py (Diff revision 5)
       
       
      Show all issues

      """ on the next line.

    23. 
        
    SS
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          rbtools/clients/tests.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          rbtools/clients/tests.py
      
      
    2. rbtools/clients/tests.py (Diff revision 6)
       
       
      Show all issues
      Col: 50
       E127 continuation line over-indented for visual indent
      
    3. rbtools/clients/tests.py (Diff revision 6)
       
       
      Show all issues
      Col: 50
       E127 continuation line over-indented for visual indent
      
    4. rbtools/clients/tests.py (Diff revision 6)
       
       
      Show all issues
      Col: 50
       E127 continuation line over-indented for visual indent
      
    5. rbtools/clients/tests.py (Diff revision 6)
       
       
      Show all issues
      Col: 50
       E127 continuation line over-indented for visual indent
      
    6. rbtools/clients/tests.py (Diff revision 6)
       
       
      Show all issues
      Col: 33
       E128 continuation line under-indented for visual indent
      
    7. rbtools/clients/tests.py (Diff revision 6)
       
       
      Show all issues
      Col: 33
       E128 continuation line under-indented for visual indent
      
    8. 
        
    SS
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          rbtools/clients/tests.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          rbtools/clients/tests.py
      
      
    2. rbtools/clients/tests.py (Diff revision 7)
       
       
      Show all issues
      Col: 33
       E128 continuation line under-indented for visual indent
      
    3. rbtools/clients/tests.py (Diff revision 7)
       
       
      Show all issues
      Col: 33
       E128 continuation line under-indented for visual indent
      
    4. 
        
    david
    1. Going to make some small changes and push this.

    2. 
        
    SS
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-0.7.x (d8a346f)