Summary

Work around different versions of patch in unit tests

Review Request #10208 — Created Oct. 9, 2018 and submitted Oct. 11, 2018, 12:30 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

Commit

48a0ed7...

Reviewers

Groups

reviewboard

People

Description

Review request https://reviews.reviewboard.org/r/10164/ landed on
release-4.0.x, but the underlying issue was introduced on release-3.0.x.
This patch is a backport of that patch. The original commit message
follows:

We had a unit test that depended on the specific version of patch
installed. Newer versions of patch will accept a diff that contains no
hunks, whereas older versions will fail with a message saying that the
input contained only garbage.

We now have unit tests that check the correct behaviour happens in both
cases by replacing our call to patch.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
Summary and description should be updated too.	chipx86	Oct. 11, 2018, 12:17 p.m.
Why the "patch(1)"?	chipx86	Oct. 11, 2018, 11:56 a.m.
We should be using keyword arguments for all PatchError calls. It's a complicated bunch of parameters.	chipx86	Oct. 9, 2018, 1:50 p.m.

flake8 passed.

JSHint passed.

reviewboard/diffviewer/tests/test_diffutils.py (Diff revision 1)

The issue has been resolved. Show all issues

Why the "patch(1)"?

chipx86 Oct. 9, 2018, 12:39 p.m.

I'm assuming this is to differentiate patch files from the patch binary the way man would do it, but that wouldn't be appropriate here. I'd say "with GNU patch ..." instead.

brennie Oct. 9, 2018, 1:49 p.m.

We have no guarantee that it will be gnu patch. It could be BSD patch (im sure somewhere out there a dev is using review board on BSD)

Also the other patch already has this. :/

chipx86 Oct. 11, 2018, 11:54 a.m.

Let's say "patch tool" then. The other one can be fixed in merge.

reviewboard/diffviewer/tests/test_diffutils.py (Diff revision 1)

The issue has been resolved. Show all issues

We should be using keyword arguments for all PatchError calls. It's a complicated bunch of parameters.

Change Summary:

Addressed feedback.

Commit:

-	7bf4a28967e66e3077c0abf8dd3c1197ba78c113
+	0c3b031e6bf22d35b4b0a983c6bbe1c07c7a3956

Diff:

Revision 2 (+93 -6)

Show changes

reviewboard/diffviewer/tests/test_diffutils.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Commit:

-	0c3b031e6bf22d35b4b0a983c6bbe1c07c7a3956
+	48a0ed7183092fe9562dbb8be419404c6030e66b

Diff:

Revision 3 (+93 -6)

Show changes

reviewboard/diffviewer/tests/test_diffutils.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

The issue has been resolved. Show all issues

Summary and description should be updated too.

Summary:

-	Work around different versions of patch(1) in unit tests
+	Work around different versions of patch in unit tests

Description:

		Review request https://reviews.reviewboard.org/r/10164/ landed on
		release-4.0.x, but the underlying issue was introduced on release-3.0.x.
		This patch is a backport of that patch. The original commit message
		follows:

~		We had a unit test that depended on the specific version of `patch(1)`
	~	We had a unit test that depended on the specific version of `patch`
		installed. Newer versions of patch will accept a diff that contains no
		hunks, whereas older versions will fail with a message saying that the
		input contained only garbage.

		We now have unit tests that check the correct behaviour happens in both
~		cases by replacing our call to `patch(1)`.
	~	cases by replacing our call to `patch`.

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (156c7ce)