Re: [PATCH 1/3] t6010: test actual test output

2017-09-15 Thread Jeff King
On Fri, Sep 15, 2017 at 12:01:27PM +0200, Michael J Gruber wrote:

> Jeff King venit, vidit, dixit 14.09.2017 16:34:
> > On Thu, Sep 14, 2017 at 03:15:18PM +0200, Michael J Gruber wrote:
> > 
> >> 4f21454b55 ("merge-base: handle --fork-point without reflog",
> >> 2016-10-12) introduced a fix for merge-base --fork-point without reflog
> >> and a test. While that test is fine, it did not update expected nor actual
> >> output and tested that of the previous test instead.
> > 
> > Oops. The use of the existing "expect3" was intentional (the idea being
> > that we are repeating the identical step from the previous test with the
> > slight tweak of not having a reflog). But obviously failing to write to
> > "actual" at all was a mistake.
> > 
> > That said, I'm OK with reiterating the expected output.
> 
> expect3 had a different content, too ;)

Oh, then it was just horribly broken, then. Oops. :)

Thanks for fixing.

-Peff


Re: [PATCH 1/3] t6010: test actual test output

2017-09-15 Thread Michael J Gruber
Jeff King venit, vidit, dixit 14.09.2017 16:34:
> On Thu, Sep 14, 2017 at 03:15:18PM +0200, Michael J Gruber wrote:
> 
>> 4f21454b55 ("merge-base: handle --fork-point without reflog",
>> 2016-10-12) introduced a fix for merge-base --fork-point without reflog
>> and a test. While that test is fine, it did not update expected nor actual
>> output and tested that of the previous test instead.
> 
> Oops. The use of the existing "expect3" was intentional (the idea being
> that we are repeating the identical step from the previous test with the
> slight tweak of not having a reflog). But obviously failing to write to
> "actual" at all was a mistake.
> 
> That said, I'm OK with reiterating the expected output.

expect3 had a different content, too ;)

Michael


Re: [PATCH 1/3] t6010: test actual test output

2017-09-14 Thread Jeff King
On Thu, Sep 14, 2017 at 03:15:18PM +0200, Michael J Gruber wrote:

> 4f21454b55 ("merge-base: handle --fork-point without reflog",
> 2016-10-12) introduced a fix for merge-base --fork-point without reflog
> and a test. While that test is fine, it did not update expected nor actual
> output and tested that of the previous test instead.

Oops. The use of the existing "expect3" was intentional (the idea being
that we are repeating the identical step from the previous test with the
slight tweak of not having a reflog). But obviously failing to write to
"actual" at all was a mistake.

That said, I'm OK with reiterating the expected output.

-Peff


[PATCH 1/3] t6010: test actual test output

2017-09-14 Thread Michael J Gruber
4f21454b55 ("merge-base: handle --fork-point without reflog",
2016-10-12) introduced a fix for merge-base --fork-point without reflog
and a test. While that test is fine, it did not update expected nor actual
output and tested that of the previous test instead.

Fix the test to test the merge-base output, not just its return code.

Signed-off-by: Michael J Gruber 
---
 t/t6010-merge-base.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh
index 31db7b5f91..17fffd7998 100755
--- a/t/t6010-merge-base.sh
+++ b/t/t6010-merge-base.sh
@@ -262,8 +262,9 @@ test_expect_success 'using reflog to find the fork point' '
 
 test_expect_success '--fork-point works with empty reflog' '
git -c core.logallrefupdates=false branch no-reflog base &&
-   git merge-base --fork-point no-reflog derived &&
-   test_cmp expect3 actual
+   git rev-parse base >expect &&
+   git merge-base --fork-point no-reflog derived >actual &&
+   test_cmp expect actual
 '
 
 test_expect_success 'merge-base --octopus --all for complex tree' '
-- 
2.14.1.712.gda4591c8a2