From 99e37c2560ecb1cc44bb743a2e4aaaedb5da1a90 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 13 Mar 2019 13:24:15 +0100
Subject: [PATCH] git p4 test: use 'test_atexit' to kill p4d and the watchdog
 process
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 <johannes.schindelin@gmx.de>
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/lib-git-p4.sh                            | 10 +++++++++-
 t/t9800-git-p4-basic.sh                    |  4 ----
 t/t9801-git-p4-branch.sh                   |  4 ----
 t/t9802-git-p4-filetype.sh                 |  4 ----
 t/t9803-git-p4-shell-metachars.sh          |  4 ----
 t/t9804-git-p4-label.sh                    |  4 ----
 t/t9805-git-p4-skip-submit-edit.sh         |  4 ----
 t/t9806-git-p4-options.sh                  |  5 -----
 t/t9807-git-p4-submit.sh                   |  4 ----
 t/t9808-git-p4-chdir.sh                    |  4 ----
 t/t9809-git-p4-client-view.sh              |  4 ----
 t/t9810-git-p4-rcs.sh                      |  4 ----
 t/t9811-git-p4-label-import.sh             |  5 -----
 t/t9812-git-p4-wildcards.sh                |  4 ----
 t/t9813-git-p4-preserve-users.sh           |  4 ----
 t/t9814-git-p4-rename.sh                   |  4 ----
 t/t9815-git-p4-submit-fail.sh              |  4 ----
 t/t9816-git-p4-locked.sh                   |  4 ----
 t/t9817-git-p4-exclude.sh                  |  4 ----
 t/t9818-git-p4-block.sh                    |  4 ----
 t/t9819-git-p4-case-folding.sh             |  4 ----
 t/t9820-git-p4-editor-handling.sh          |  4 ----
 t/t9821-git-p4-path-variations.sh          |  4 ----
 t/t9822-git-p4-path-encoding.sh            |  4 ----
 t/t9823-git-p4-mock-lfs.sh                 |  4 ----
 t/t9824-git-p4-git-lfs.sh                  |  4 ----
 t/t9825-git-p4-handle-utf16-without-bom.sh |  4 ----
 t/t9826-git-p4-keep-empty-commits.sh       |  4 ----
 t/t9827-git-p4-change-filetype.sh          |  4 ----
 t/t9828-git-p4-map-user.sh                 |  4 ----
 t/t9829-git-p4-jobs.sh                     |  4 ----
 t/t9830-git-p4-symlink-dir.sh              |  4 ----
 t/t9831-git-p4-triggers.sh                 |  4 ----
 t/t9832-unshelve.sh                        |  3 ---
 t/t9833-errors.sh                          |  5 -----
 35 files changed, 9 insertions(+), 139 deletions(-)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index b3be3ba011a..958e33b77ee 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -74,7 +74,6 @@ 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
@@ -82,7 +81,16 @@ trap cleanup EXIT
 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
index 729cd257701..5856563068c 100755
--- 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
index 6a86d6996b9..50013132c85 100755
--- 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
index 9978352d782..94edebe2726 100755
--- 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
index d5c36751004..2913277013d 100755
--- 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
index e30f80e6176..32364571063 100755
--- 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
index 5fbf904dc8f..90ef647db7e 100755
--- 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
index 3f5291b8575..4e794a01bf5 100755
--- 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
index 850d9791196..eaaae414a1f 100755
--- 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
index 11d2b5102cd..58a9b3b71e6 100755
--- 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
index 897b3c3034e..3cff1fce1b7 100755
--- 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
index cc53debe195..57b533dc6fb 100755
--- 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
index 602b0a5d5ce..b70e81c3cd2 100755
--- 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
index 0206771fbb9..254a7c24469 100755
--- 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
index 783c6ad1653..fd018c87a80 100755
--- 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
index 60baa06e27a..468767cbf4b 100755
--- 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
index eaf03a65632..9779dc0d11f 100755
--- 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
index d048bd33fa3..932841003cf 100755
--- 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
index aac568eadfc..96d25f0c02c 100755
--- 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
index ce7cb22ad37..0db7ab99184 100755
--- 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
index d808c008c11..600ce1e0b0d 100755
--- 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
index 3c22f74bd43..fa1bba1dd93 100755
--- 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
index 81e46acfa8b..ef80f1690bc 100755
--- 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
index c78477c19b4..1bf7635016f 100755
--- 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
index 1f2dc369bfa..88b76dc4d6c 100755
--- 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
index ed80ca858c8..a28dbbdd566 100755
--- 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
index 1551845dc17..f049ff8229c 100755
--- 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
index fa8b9daf1fa..fd64afe064e 100755
--- 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
index 7433998f477..d3670bd7a24 100755
--- 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
index e20395c89f3..ca6c2942bdf 100755
--- 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
index 971aeeea1fb..88cfb1fcd3f 100755
--- 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
index 2ad1b0810df..3fb6960c18f 100755
--- 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
index be44c9751ae..d743ca33ee6 100755
--- 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
index 41c09f11f4b..1286a5b824b 100755
--- 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
index 47b312e1c97..e22369ccdf5 100755
--- 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