Inline text files in unit tests
Review Request #9971 — Created May 23, 2018 and submitted
Instead of the base test case opening and closing the same files
multiple times we have now moved the files inline with the code
resulting in a lower time complexity and a more streamline code design.
Re-ran the unit tests to make sure they behaved as expected.
Description | From | Last Updated |
---|---|---|
Can you delete the files you inlined? There is no reason to keep them in the repository. |
|
|
Testing done -- did you run unit tests? :) |
|
|
Can you wrap your description at 72 instead of (what I assume is) 52? |
|
|
Extra space in your change description: "we have now<2 spaces>moved the files ..." |
|
|
Can you suffix all these with _DIFF ? |
|
|
Can you extract this into a variable like the others? |
|
|
Can you pull this out into a variable and do {'path': diff} ? |
|
|
Same here. |
|
|
Same here. |
|
|
Same here. |
|
|
Same here. |
|
|
Same here. |
|
|
Same here. |
|
|
Same here. |
|
|
Same here. |
|
|
Same here. |
|
|
Same here. |
|
|
Same here. |
|
|
Same here. |
|
|
Same here. |
|
|
Same here. |
|
|
You can set diff= as a parameter to Commit. |
|
|
You can set diff= as a parameter to Commit. |
|
|
It's best not to change lines that aren't related to the changes in the file. Makes it hard to go … |
|
|
While nice, this cleanup also doesn't belong in this change. |
|
|
We have a lot of variations in how we're creating this. Can you go through the change and make sure … |
|
-
-
-
-
-
-
reviewboard/webapi/tests/test_diff.py (Diff revision 1) Can you pull this out into a variable and do
{'path': diff}
? -
-
-
-
-
-
-
-
-
-
-
-
-
-
Description: |
|
|||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
|||||||||||||||||||||
Commit: |
|
|||||||||||||||||||||
Diff: |
Revision 2 (+339 -327) |
Checks run (1 succeeded, 1 failed with error)
-
Minor nit then this looks good to go!
-
reviewboard/webapi/tests/test_diff.py (Diff revisions 1 - 2) Can you extract this into a variable like the others?
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+340 -327) |
Checks run (1 succeeded, 1 failed with error)
-
-
-
reviewboard/reviews/tests/test_review_request_draft.py (Diff revision 3) You can set
diff=
as a parameter toCommit
. -
reviewboard/reviews/tests/test_review_request_draft.py (Diff revision 3) You can set
diff=
as a parameter toCommit
. -
reviewboard/webapi/tests/test_draft_diff.py (Diff revision 3) It's best not to change lines that aren't related to the changes in the file. Makes it hard to go through the history of code later ("I wanted to see when this docstring was added -- why was it changed in this change??"). Can you revert the docstring/line wrapping changes?
-
reviewboard/webapi/tests/test_draft_diff.py (Diff revision 3) While nice, this cleanup also doesn't belong in this change.
-
reviewboard/webapi/tests/test_validate_diff.py (Diff revision 3) We have a lot of variations in how we're creating this. Can you go through the change and make sure we always pull out
diff = SimpleUploadedFile
so it's explicitly done separately from the post operation?
Description: |
|
||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
||||||||||||
Diff: |
Revision 4 (+310 -292) |