git p4 test: use 'test_atexit' to kill p4d and the watchdog process

dscho · gitster · commit 99e37c2560ec · 2019-03-14T12:34:39.000+09:00
Use 'test_atexit' to run cleanup commands to stop 'p4d' at the end of
the test script or upon interrupt or failure, as it is shorter,
simpler, and more robust than registering such cleanup commands in the
trap on EXIT in the test scripts.

Note that one of the test scripts, 't9801-git-p4-branch.sh', stops and
then re-starts 'p4d' twice in the middle of the script; take care that
the cleanup functions to stop 'p4d' are only registered once.

Note also that 'git p4' tests invoke different functions in the trap
on EXIT ('cleanup') and in the last test before 'test_done'
('kill_p4d').  Register both of these functions with 'test_atexit' for
now, and a a later patch in this series will then clean up the
redundancy.

Signed-off-by: Johannes Schindelin &lt;johannes.schindelin@gmx.de&gt;
Signed-off-by: SZEDER Gábor &lt;szeder.dev@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
@@ -74,15 +74,23 @@ cleanup () {
 		kill -9 $(cat "$pidfile") 2>/dev/null && exit 255
 	fi
 }
-trap cleanup EXIT
 
 # git p4 submit generates a temp file, which will
 # not get cleaned up if the submission fails.  Don't
 # clutter up /tmp on the test machine.
 TMPDIR="$TRASH_DIRECTORY"
 export TMPDIR
 
+registered_stop_p4d_atexit_handler=
 start_p4d () {
+	# One of the test scripts stops and then re-starts p4d.
+	# Don't register and then run the same atexit handlers several times.
+	if test -z "$registered_stop_p4d_atexit_handler"
+	then
+		test_atexit 'kill_p4d; cleanup'
+		registered_stop_p4d_atexit_handler=AlreadyDone
+	fi
+
 	mkdir -p "$db" "$cli" "$git" &&
 	rm -f "$pidfile" &&
 	(
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
@@ -326,8 +326,4 @@ test_expect_success 'submit from worktree' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
@@ -610,8 +610,4 @@ test_expect_success 'Update a file in git side and submit to P4 using client vie
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
@@ -333,8 +333,4 @@ test_expect_success SYMLINKS 'empty symlink target' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9803-git-p4-shell-metachars.sh b/t/t9803-git-p4-shell-metachars.sh
@@ -105,8 +105,4 @@ test_expect_success 'branch with shell char' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9804-git-p4-label.sh b/t/t9804-git-p4-label.sh
@@ -108,8 +108,4 @@ test_expect_failure 'two labels on the same changelist' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9805-git-p4-skip-submit-edit.sh b/t/t9805-git-p4-skip-submit-edit.sh
@@ -98,8 +98,4 @@ test_expect_success 'no config, edited' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
@@ -300,9 +300,4 @@ test_expect_success 'use --git-dir option and GIT_DIR' '
 	test_path_is_file "$git"/cli_file2.t
 '
 
-
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
@@ -593,8 +593,4 @@ test_expect_success 'update a shelve involving moved and copied files' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9808-git-p4-chdir.sh b/t/t9808-git-p4-chdir.sh
@@ -83,8 +83,4 @@ test_expect_success SYMLINKS 'p4 client root symlink should stay symbolic' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
@@ -836,8 +836,4 @@ test_expect_success 'quotes on both sides' '
 	git_verify "cdir 1/file11" "cdir 1/file12"
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh
@@ -360,8 +360,4 @@ test_expect_failure 'Add keywords in git which do not match the default p4 value
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh
@@ -259,9 +259,4 @@ test_expect_success 'importing labels with missing revisions' '
 	)
 '
 
-
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9812-git-p4-wildcards.sh b/t/t9812-git-p4-wildcards.sh
@@ -211,8 +211,4 @@ test_expect_success 'wildcard files requiring keyword scrub' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
@@ -138,8 +138,4 @@ test_expect_success 'not preserving user with mixed authorship' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
@@ -242,8 +242,4 @@ test_expect_success P4D_HAVE_CONFIGURABLE_RUN_MOVE_ALLOW \
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh
@@ -422,8 +422,4 @@ test_expect_success 'cleanup chmod after submit cancel' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
@@ -138,8 +138,4 @@ test_expect_failure 'move with lock taken' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9817-git-p4-exclude.sh b/t/t9817-git-p4-exclude.sh
@@ -64,8 +64,4 @@ test_expect_success 'clone, then sync with exclude' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
@@ -146,8 +146,4 @@ test_expect_success 'Clone repo with self-sizing block size' '
 	test_line_count \> 10 log
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9819-git-p4-case-folding.sh b/t/t9819-git-p4-case-folding.sh
@@ -53,8 +53,4 @@ test_expect_failure 'Clone UC repo with lc name' '
 	test_must_fail git p4 clone //depot/uc/...
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9820-git-p4-editor-handling.sh b/t/t9820-git-p4-editor-handling.sh
@@ -31,8 +31,4 @@ test_expect_success 'EDITOR with options' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9821-git-p4-path-variations.sh b/t/t9821-git-p4-path-variations.sh
@@ -193,8 +193,4 @@ test_expect_success 'Add a new file and clone path with new file (ignorecase)' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
@@ -67,8 +67,4 @@ test_expect_success 'Delete iso8859-1 encoded paths and clone' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9823-git-p4-mock-lfs.sh b/t/t9823-git-p4-mock-lfs.sh
@@ -185,8 +185,4 @@ test_expect_success 'Run git p4 submit in repo configured with large file system
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9824-git-p4-git-lfs.sh b/t/t9824-git-p4-git-lfs.sh
@@ -287,8 +287,4 @@ test_expect_success 'Add big files to repo and store files in LFS based on compr
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9825-git-p4-handle-utf16-without-bom.sh b/t/t9825-git-p4-handle-utf16-without-bom.sh
@@ -43,8 +43,4 @@ test_expect_failure 'clone depot with invalid UTF-16 file in non-verbose mode' '
 	git p4 clone --dest="$git" //depot
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9826-git-p4-keep-empty-commits.sh b/t/t9826-git-p4-keep-empty-commits.sh
@@ -127,8 +127,4 @@ test_expect_success 'Clone repo subdir with all history' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9827-git-p4-change-filetype.sh b/t/t9827-git-p4-change-filetype.sh
@@ -59,8 +59,4 @@ test_expect_success SYMLINKS 'change symbolic link to file' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9828-git-p4-map-user.sh b/t/t9828-git-p4-map-user.sh
@@ -54,8 +54,4 @@ test_expect_success 'Clone repo root path with all history' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9829-git-p4-jobs.sh b/t/t9829-git-p4-jobs.sh
@@ -92,8 +92,4 @@ test_expect_success 'check log message of changelist with more jobs' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9830-git-p4-symlink-dir.sh b/t/t9830-git-p4-symlink-dir.sh
@@ -36,8 +36,4 @@ test_expect_success 'symlinked directory' '
 
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9831-git-p4-triggers.sh b/t/t9831-git-p4-triggers.sh
@@ -96,8 +96,4 @@ test_expect_success 'submit description with extra info lines from verbose p4 ch
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
 test_done
diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh
@@ -174,8 +174,5 @@ test_expect_success 'unshelve specifying the origin' '
 		test_path_is_file file_to_shelve
 	)
 '
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
 
 test_done
diff --git a/t/t9833-errors.sh b/t/t9833-errors.sh
@@ -45,9 +45,4 @@ test_expect_success 'ticket logged out' '
 	)
 '
 
-test_expect_success 'kill p4d' '
-	kill_p4d
-'
-
-
 test_done

Original file line number	Diff line number	Diff line change
`@@ -326,8 +326,4 @@ test_expect_success 'submit from worktree' '`
`326`	`326`	`)`
`327`	`327`	`'`
`328`	`328`
`329`		`-test_expect_success 'kill p4d' '`
`330`		`- kill_p4d`
`331`		`-'`
`332`		`-`
`333`	`329`	`test_done`
Original file line number	Diff line number	Diff line change
`@@ -610,8 +610,4 @@ test_expect_success 'Update a file in git side and submit to P4 using client vie`
`610`	`610`	`)`
`611`	`611`	`'`
`612`	`612`
`613`		`-test_expect_success 'kill p4d' '`
`614`		`- kill_p4d`
`615`		`-'`
`616`		`-`
`617`	`613`	`test_done`
Original file line number	Diff line number	Diff line change
`@@ -333,8 +333,4 @@ test_expect_success SYMLINKS 'empty symlink target' '`
`333`	`333`	`)`
`334`	`334`	`'`
`335`	`335`
`336`		`-test_expect_success 'kill p4d' '`
`337`		`- kill_p4d`
`338`		`-'`
`339`		`-`
`340`	`336`	`test_done`
Original file line number	Diff line number	Diff line change
`@@ -105,8 +105,4 @@ test_expect_success 'branch with shell char' '`
`105`	`105`	`)`
`106`	`106`	`'`
`107`	`107`
`108`		`-test_expect_success 'kill p4d' '`
`109`		`- kill_p4d`
`110`		`-'`
`111`		`-`
`112`	`108`	`test_done`
Original file line number	Diff line number	Diff line change
`@@ -108,8 +108,4 @@ test_expect_failure 'two labels on the same changelist' '`
`108`	`108`	`)`
`109`	`109`	`'`
`110`	`110`
`111`		`-test_expect_success 'kill p4d' '`
`112`		`- kill_p4d`
`113`		`-'`
`114`		`-`
`115`	`111`	`test_done`
Original file line number	Diff line number	Diff line change
`@@ -98,8 +98,4 @@ test_expect_success 'no config, edited' '`
`98`	`98`	`)`
`99`	`99`	`'`
`100`	`100`
`101`		`-test_expect_success 'kill p4d' '`
`102`		`- kill_p4d`
`103`		`-'`
`104`		`-`
`105`	`101`	`test_done`
Original file line number	Diff line number	Diff line change
`@@ -300,9 +300,4 @@ test_expect_success 'use --git-dir option and GIT_DIR' '`
`300`	`300`	`test_path_is_file "$git"/cli_file2.t`
`301`	`301`	`'`
`302`	`302`
`303`		`-`
`304`		`-test_expect_success 'kill p4d' '`
`305`		`- kill_p4d`
`306`		`-'`
`307`		`-`
`308`	`303`	`test_done`
Original file line number	Diff line number	Diff line change
`@@ -593,8 +593,4 @@ test_expect_success 'update a shelve involving moved and copied files' '`
`593`	`593`	`)`
`594`	`594`	`'`
`595`	`595`
`596`		`-test_expect_success 'kill p4d' '`
`597`		`- kill_p4d`
`598`		`-'`
`599`		`-`
`600`	`596`	`test_done`
Original file line number	Diff line number	Diff line change
`@@ -83,8 +83,4 @@ test_expect_success SYMLINKS 'p4 client root symlink should stay symbolic' '`
`83`	`83`	`)`
`84`	`84`	`'`
`85`	`85`
`86`		`-test_expect_success 'kill p4d' '`
`87`		`- kill_p4d`
`88`		`-'`
`89`		`-`
`90`	`86`	`test_done`