Re: [PATCH] t6026: ensure that long-running script really is
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Nov 11, 2016, 06:50 UTC
- Message-ID
- <fbf517ad-7341-eb6d-ab38-4fe91410e57c@kdbg.org>
- In-Reply-To
- <xmqqfumy51tk.fsf@gitster.mtv.corp.google.com>
Am 11.11.2016 um 00:53 schrieb Junio C Hamano:
Show 30 quoted lines
> Johannes Schindelin <johannes.schindelin@gmx.de> writes: > >> When making sure that background tasks are cleaned up in 5babb5b >> (t6026-merge-attr: clean up background process at end of test case, >> 2016-09-07), we considered to let the background task sleep longer, just >> to be certain that it will still be running when we want to kill it >> after the test. >> >> Sadly, the assumption appears not to hold true that the test case passes >> quickly enough to kill the background task within a second. >> >> Simply increase it to an hour. No system can be possibly slow enough to >> make above-mentioned assumption incorrect. >> >> Reported by Andreas Schwab. >> >> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> >> --- >> Published-As: https://github.com/dscho/git/releases/tag/t6026-sleep-v1 >> Fetch-It-Via: git fetch https://github.com/dscho/git t6026-sleep-v1 > > OK, I think this is a much better option. Assuming 3600 is long > enough for everybody (and if not, we have a bigger problem ;-), > it will ensure that the stray process will be around when we run the > 'git merge' test, and by not adding "|| :" after the kill, we check > that the stray process is still there, i.e. we tested what we wanted > to test. > > Will revert the two patches that were queued previously and then > queue this one.
Shouldn't we then move the 'kill' out of test_when_finished and make it a proper condition of the test? Like this?
diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh index 348d78b205..6ad8bd098a 100755 --- a/t/t6026-merge-attr.sh +++ b/t/t6026-merge-attr.sh @@ -187,13 +187,19 @@ test_expect_success 'custom merge does not lock index' ' sleep 3600 & echo $! >sleep.pid EOF - test_when_finished "kill \$(cat sleep.pid)" && test_write_lines >.gitattributes \ "* merge=ours" "text merge=sleep-an-hour" && test_config merge.ours.driver true && test_config merge.sleep-an-hour.driver ./sleep-an-hour.sh && - git merge master + git merge master && + + # We are testing that the custom merge driver does not block + # index.lock on Windows due to an inherited file handle. + # To ensure that this test checks this condition, the process + # must still be running at this point (and must have started + # in the first place), hence, this kill must not fail: + kill "$(cat sleep.pid)" ' test_done