Fix diff error bundle using wrong string type on Python 3.
Review Request #11960 — Created Jan. 21, 2022 and submitted
Information | |
---|---|
kylemclean | |
Review Board | |
release-4.0.x | |
4958 | |
Reviewers | |
reviewboard | |
This change fixes bug #4958 by changing
zipfile
in
DownloadPatchErrorBundleView
to useBytesIO
instead ofStringIO
, which
makes it work on both Python 2 and 3. Before this change, the view would fail
when Python 3 was used.This change also adds two unit tests for
DownloadPatchErrorBundleView
that test
its response when there is or isn't aPatchError
raised in rendering the diff.
A review request with a patch that does not apply cleanly was created.
The patch error bundle was successfully downloaded when running
the Review Board dev server on both Python 2 and 3.Two unit tests were added for
DownloadPatchErrorBundleView
that test its
responses. It was verified that these unit tests fail onrelease-4.0.x
when Python 3 is used, and pass on both Python 2 and 3 after this change.All unit tests in
reviewboard.diffviewer.tests
were run and passed on
both Python 2 and 3.
Summary | |
---|---|
Description | From | Last Updated |
---|---|---|
Please add the bug number to the "Bugs" field. |
|
|
Can we add a unit test for this? |
|
|
E501 line too long (83 > 79 characters) |
![]() |
|
W292 no newline at end of file |
![]() |
|
Imports in Python are in 3 groups, each separated by a blank line: Standard library imports Third-party modules (from the … |
|
|
Docstring summaries must be one line. If this can't fit, just simplify the reference to DownloadPatchErrorBundleView. |
|
|
The trailing """ must be on its own line. Same below. |
|
|
This can be: self.spy_on(..., op=kgb.SpyOpReturn(HttpResponse())) |
|
|
Can you group this with the other from ... imports in this import group? Make sure it's grouped alphabetically. |
|
Change Summary:
Add unit tests to
DownloadPatchErrorBundleView
.
Description: |
|
||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
||||||||||||||||||||||||||||||
Commits: |
|
||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+154 -4) |
Checks run (1 failed, 1 succeeded)
flake8
-
reviewboard/diffviewer/tests/test_download_patch_error_bundle_view.py (Diff revision 2) E501 line too long (83 > 79 characters)
-
reviewboard/diffviewer/tests/test_download_patch_error_bundle_view.py (Diff revision 2) W292 no newline at end of file
Change Summary:
Fix code formatting.
Commits: |
|
|||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+156 -4) |
Checks run (2 succeeded)
-
-
reviewboard/diffviewer/tests/test_download_patch_error_bundle_view.py (Diff revision 3) Imports in Python are in 3 groups, each separated by a blank line:
- Standard library imports
- Third-party modules (from the point of view of the application)
- Application modules.
Inside of each group, we order them:
import ...
entries beforefrom ...
entries- Alphabetical within each.
-
reviewboard/diffviewer/tests/test_download_patch_error_bundle_view.py (Diff revision 3) Docstring summaries must be one line.
If this can't fit, just simplify the reference to
DownloadPatchErrorBundleView
. -
reviewboard/diffviewer/tests/test_download_patch_error_bundle_view.py (Diff revision 3) The trailing
"""
must be on its own line.Same below.
-
reviewboard/diffviewer/tests/test_download_patch_error_bundle_view.py (Diff revision 3) This can be:
self.spy_on(..., op=kgb.SpyOpReturn(HttpResponse()))
-
reviewboard/diffviewer/views.py (Diff revision 3) Can you group this with the other
from ...
imports in this import group? Make sure it's grouped alphabetically.
Change Summary:
Make requested changes from Christian Hammond's review
Commits: |
|
||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 4 (+163 -17) |