[2/1] Revert "t/lib-git-daemon: record daemon log"
diff mbox series

Message ID 20190106175310.GC25639@hank.intra.tgummerer.com
State New
Headers show
Series
  • t5570: drop racy test
Related show

Commit Message

Thomas Gummerer Jan. 6, 2019, 5:53 p.m. UTC
This reverts commit 314a73d658 (t/lib-git-daemon: record daemon log,
2018-01-25), which let tests use the output of git-daemon.

The previous commit removed the last user of deamon.log in the tests,
there's no good way to make checking for output in the log
race-proof.  Revert this commit as well, to make sure others are not
tempted to use daemon.log in tests in the future, which would lead to
racy tests.

The original commit had one change that still makes sense, namely
switching read/echo for "read -r" and "printf", which relays the data
more faithfully.  Don't revert that piece here, as it is still a
useful change.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 t/lib-git-daemon.sh | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

Comments

Jeff King Jan. 7, 2019, 8:20 a.m. UTC | #1
On Sun, Jan 06, 2019 at 05:53:10PM +0000, Thomas Gummerer wrote:

> This reverts commit 314a73d658 (t/lib-git-daemon: record daemon log,
> 2018-01-25), which let tests use the output of git-daemon.
> 
> The previous commit removed the last user of deamon.log in the tests,
> there's no good way to make checking for output in the log
> race-proof.  Revert this commit as well, to make sure others are not
> tempted to use daemon.log in tests in the future, which would lead to
> racy tests.
> 
> The original commit had one change that still makes sense, namely
> switching read/echo for "read -r" and "printf", which relays the data
> more faithfully.  Don't revert that piece here, as it is still a
> useful change.
> 
> Suggested-by: Jeff King <peff@peff.net>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>

Yep, this looks good to me. Thanks for being extra careful with the
read/printf bits!

Looks like Junio already queued a99653a9b6 (Revert "t/lib-git-daemon:
record daemon log", 2018-12-28) on the tip of tg/t5570-drop-racy-test,
but that's a pure revert. I think we can replace it with this.

-Peff
Junio C Hamano Jan. 7, 2019, 3:45 p.m. UTC | #2
Jeff King <peff@peff.net> writes:

> Yep, this looks good to me. Thanks for being extra careful with the
> read/printf bits!
>
> Looks like Junio already queued a99653a9b6 (Revert "t/lib-git-daemon:
> record daemon log", 2018-12-28) on the tip of tg/t5570-drop-racy-test,
> but that's a pure revert. I think we can replace it with this.

Thanks, both.  Will do.

Patch
diff mbox series

diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
index f98de95c15..fd41229a8f 100644
--- a/t/lib-git-daemon.sh
+++ b/t/lib-git-daemon.sh
@@ -54,19 +54,11 @@  start_git_daemon() {
 		"$@" "$GIT_DAEMON_DOCUMENT_ROOT_PATH" \
 		>&3 2>git_daemon_output &
 	GIT_DAEMON_PID=$!
-	>daemon.log
 	{
 		read -r line <&7
-		printf "%s\n" "$line"
-		printf >&4 "%s\n" "$line"
-		(
-			while read -r line <&7
-			do
-				printf "%s\n" "$line"
-				printf >&4 "%s\n" "$line"
-			done
-		) &
-	} 7<git_daemon_output >>"$TRASH_DIRECTORY/daemon.log" &&
+		printf "%s\n" "$line" >&4
+		cat <&7 >&4 &
+	} 7<git_daemon_output &&
 
 	# Check expected output
 	if test x"$(expr "$line" : "\[[0-9]*\] \(.*\)")" != x"Ready to rumble"