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)