Add new unit tests for RBTools's Git support
Review Request #8099 — Created April 4, 2016 and submitted
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_urlMethod 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 |
reviewbot | |
Col: 45 W291 trailing whitespace |
reviewbot | |
Col: 60 W291 trailing whitespace |
reviewbot | |
local variable 'e' is assigned to but never used |
reviewbot | |
Col: 45 W291 trailing whitespace |
reviewbot | |
Col: 1 W293 blank line contains whitespace |
reviewbot | |
Col: 5 E303 too many blank lines (2) |
reviewbot | |
Col: 63 W291 trailing whitespace |
reviewbot | |
Col: 63 W291 trailing whitespace |
reviewbot | |
Col: 45 W291 trailing whitespace |
reviewbot | |
Col: 58 W291 trailing whitespace |
reviewbot | |
Col: 63 W291 trailing whitespace |
reviewbot | |
Col: 77 W291 trailing whitespace |
reviewbot | |
Col: 45 W291 trailing whitespace |
reviewbot | |
Col: 64 W291 trailing whitespace |
reviewbot | |
Col: 76 W291 trailing whitespace |
reviewbot | |
Col: 32 E711 comparison to None should be 'if cond is not None:' |
reviewbot | |
Col: 65 W291 trailing whitespace |
reviewbot | |
Col: 75 W291 trailing whitespace |
reviewbot | |
Col: 68 W291 trailing whitespace |
reviewbot | |
Col: 32 E711 comparison to None should be 'if cond is not None:' |
reviewbot | |
Col: 45 W291 trailing whitespace |
reviewbot | |
Col: 60 E231 missing whitespace after ':' |
reviewbot | |
Col: 68 W291 trailing whitespace |
reviewbot | |
Col: 57 E231 missing whitespace after ':' |
reviewbot | |
Col: 60 E231 missing whitespace after ':' |
reviewbot | |
Col: 68 W291 trailing whitespace |
reviewbot | |
Col: 57 E231 missing whitespace after ':' |
reviewbot | |
Col: 60 E231 missing whitespace after ':' |
reviewbot | |
Col: 68 W291 trailing whitespace |
reviewbot | |
Col: 57 E231 missing whitespace after ':' |
reviewbot | |
Col: 33 E128 continuation line under-indented for visual indent |
reviewbot | |
Col: 60 E231 missing whitespace after ':' |
reviewbot | |
Col: 68 W291 trailing whitespace |
reviewbot | |
Col: 57 E231 missing whitespace after ':' |
reviewbot | |
Col: 33 E128 continuation line under-indented for visual indent |
reviewbot | |
You could put this after the push_upstream call on line 538 - that way, you wouldn't need the early return … |
mike_conley | |
Don't forget to test the success case too. |
mike_conley | |
assertTrue(False) after line 551? Same goes for other tests like this. |
mike_conley | |
Col: 33 E128 continuation line under-indented for visual indent |
reviewbot | |
Col: 33 E128 continuation line under-indented for visual indent |
reviewbot | |
I'm guessing this is just placeholder? |
mike_conley | |
Col: 50 E127 continuation line over-indented for visual indent |
reviewbot | |
Col: 50 E127 continuation line over-indented for visual indent |
reviewbot | |
Col: 50 E127 continuation line over-indented for visual indent |
reviewbot | |
Col: 50 E127 continuation line over-indented for visual indent |
reviewbot | |
Col: 33 E128 continuation line under-indented for visual indent |
reviewbot | |
Col: 33 E128 continuation line under-indented for visual indent |
reviewbot | |
Col: 76 W291 trailing whitespace |
reviewbot | |
""" on the next line. |
david | |
This could all be done at once using six.assertRaisesRegexp. |
david | |
Instead of catching the exception and doing assertTrue(False), just get rid of the try/except. An uncaught exception will cause the … |
david | |
""" on the next line. |
david | |
This can use six.assertRaisesRegexp |
david | |
""" on the next line. |
david | |
Can we reformat/change this to: author = type( b'Author', (object,), { 'fullname': 'name', 'email': 'email', }) |
david | |
six.assertRaisesMessage |
david | |
""" on the next line. |
david | |
If you add SpyAgency as a parent class, you can use self.spy_on(execute). This has several advantages: you don't have to … |
david | |
Same formatting here as I mentioned above. |
david | |
Col: 50 E127 continuation line over-indented for visual indent |
reviewbot | |
Col: 50 E127 continuation line over-indented for visual indent |
reviewbot | |
""" on the next line. |
david | |
Col: 50 E127 continuation line over-indented for visual indent |
reviewbot | |
Col: 50 E127 continuation line over-indented for visual indent |
reviewbot | |
""" on the next line. |
david | |
Same formatting here as above. |
david | |
Col: 33 E128 continuation line under-indented for visual indent |
reviewbot | |
""" on the next line. |
david | |
And here. |
david | |
Col: 33 E128 continuation line under-indented for visual indent |
reviewbot | |
""" on the next line. |
david | |
And here. Since we're using this in lots of places, perhaps we should define this once? |
david | |
""" on the next line. |
david | |
""" on the next line. |
david | |
""" on the next line. |
david | |
Col: 50 E127 continuation line over-indented for visual indent |
reviewbot | |
Col: 50 E127 continuation line over-indented for visual indent |
reviewbot | |
Col: 50 E127 continuation line over-indented for visual indent |
reviewbot | |
Col: 50 E127 continuation line over-indented for visual indent |
reviewbot | |
Col: 33 E128 continuation line under-indented for visual indent |
reviewbot | |
Col: 33 E128 continuation line under-indented for visual indent |
reviewbot | |
Col: 33 E128 continuation line under-indented for visual indent |
reviewbot | |
Col: 33 E128 continuation line under-indented for visual indent |
reviewbot |
- Change Summary:
-
Fixed some stylistic issues, Added tests for create_commit and delete_branch which use kgb spies.
- Diff:
-
Revision 2 (+164)
-
Tool: Pyflakes Processed Files: rbtools/clients/tests.py Tool: PEP8 Style Checker Processed Files: rbtools/clients/tests.py
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
- Change Summary:
-
Edited the Testing Done field
- Testing Done:
-
~ Wrote some test cases for the merge, push_upstream and create_commit methods (some of these test cases are still incomplete because I'm still learning how to use kgb spies).
~ Wrote some test cases for the merge, push_upstream, delete_branch and create_commit methods.
- Change Summary:
-
Added merge tests (squash - true/false) and create_commit tests (all_files - true/false). Also fixed issues opened by Mike
- Diff:
-
Revision 4 (+278)
-
Tool: Pyflakes Processed Files: rbtools/clients/tests.py Tool: PEP8 Style Checker Processed Files: rbtools/clients/tests.py
-
-
-
-
-
-
-
- Change Summary:
-
Add a new test case for push_upstream, removed an old one, and finalized these set of tests to be non-WIP
- Summary:
-
[WIP] Add new unit tests for RBTools's Git supportAdd new unit tests for RBTools's Git support
- Description:
-
~ [WIP] Add new unit tests for RBTools's Git support
~ 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)
- Testing Done:
-
~ Wrote some test cases for the merge, push_upstream, delete_branch and create_commit methods.
~ 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. - Diff:
-
Revision 5 (+281)
-
Tool: Pyflakes Processed Files: rbtools/clients/tests.py Tool: PEP8 Style Checker Processed Files: rbtools/clients/tests.py
-
-
-
-
-
-
-
-
-
-
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. -
-
-
-
Can we reformat/change this to:
author = type( b'Author', (object,), { 'fullname': 'name', 'email': 'email', })
-
-
-
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). -
-
-
-
-
-
-
-
-
-
-
-
Tool: Pyflakes Processed Files: rbtools/clients/tests.py Tool: PEP8 Style Checker Processed Files: rbtools/clients/tests.py
-
-
-
-
-
-