From 37eaab2f76341d6a4dd253b67b1567c807c2e219 Mon Sep 17 00:00:00 2001
From: Derrick Stolee <derrickstolee@github.com>
Date: Tue, 20 Aug 2024 22:45:13 -0400
Subject: [PATCH 1/7] t1000-2000: add GIT_ADVICE=1 for advice tests

Several tests validate the exact output of stderr, including when the stderr
file should be empty. In advance of modifying the advice system to only
output when stderr is a terminal, force the advice system to output in these
cases.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/t1092-sparse-checkout-compatibility.sh  | 18 ++++++++--------
 t/t2020-checkout-detach.sh                | 25 ++++++++++++++---------
 t/t2024-checkout-dwim.sh                  |  5 +++--
 t/t2060-switch.sh                         |  4 ++--
 t/t2204-add-ignored.sh                    |  8 ++++----
 t/t2400-worktree-add.sh                   | 12 +++++------
 t/t7500-commit-template-squash-signoff.sh |  3 ++-
 7 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index a2c0e1b4dcc564..b5183ea7c83874 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -411,10 +411,10 @@ test_expect_success 'add outside sparse cone' '
 	run_on_sparse mkdir folder1 &&
 	run_on_sparse ../edit-contents folder1/a &&
 	run_on_sparse ../edit-contents folder1/newfile &&
-	test_sparse_match test_must_fail git add folder1/a &&
+	test_env GIT_ADVICE=1 test_sparse_match test_must_fail git add folder1/a &&
 	grep "Disable or modify the sparsity rules" sparse-checkout-err &&
 	test_sparse_unstaged folder1/a &&
-	test_sparse_match test_must_fail git add folder1/newfile &&
+	test_env GIT_ADVICE=1 test_sparse_match test_must_fail git add folder1/newfile &&
 	grep "Disable or modify the sparsity rules" sparse-checkout-err &&
 	test_sparse_unstaged folder1/newfile
 '
@@ -466,13 +466,13 @@ test_expect_success 'status/add: outside sparse cone' '
 	test_sparse_match git status --porcelain=v2 &&
 
 	# Adding the path outside of the sparse-checkout cone should fail.
-	test_sparse_match test_must_fail git add folder1/a &&
+	test_env GIT_ADVICE=1 test_sparse_match test_must_fail git add folder1/a &&
 	grep "Disable or modify the sparsity rules" sparse-checkout-err &&
 	test_sparse_unstaged folder1/a &&
 	test_all_match git add --refresh folder1/a &&
 	test_must_be_empty sparse-checkout-err &&
 	test_sparse_unstaged folder1/a &&
-	test_sparse_match test_must_fail git add folder1/new &&
+	test_env GIT_ADVICE=1 test_sparse_match test_must_fail git add folder1/new &&
 	grep "Disable or modify the sparsity rules" sparse-checkout-err &&
 	test_sparse_unstaged folder1/new &&
 	test_sparse_match git add --sparse folder1/a &&
@@ -1018,7 +1018,7 @@ test_expect_success 'merge with conflict outside cone' '
 	test_all_match git status --porcelain=v2 &&
 
 	# 2. Add the file with conflict markers
-	test_sparse_match test_must_fail git add folder1/a &&
+	test_env GIT_ADVICE=1 test_sparse_match test_must_fail git add folder1/a &&
 	grep "Disable or modify the sparsity rules" sparse-checkout-err &&
 	test_sparse_unstaged folder1/a &&
 	test_all_match git add --sparse folder1/a &&
@@ -1027,7 +1027,7 @@ test_expect_success 'merge with conflict outside cone' '
 	# 3. Rename the file to another sparse filename and
 	#    accept conflict markers as resolved content.
 	run_on_all mv folder2/a folder2/z &&
-	test_sparse_match test_must_fail git add folder2 &&
+	test_env GIT_ADVICE=1 test_sparse_match test_must_fail git add folder2 &&
 	grep "Disable or modify the sparsity rules" sparse-checkout-err &&
 	test_sparse_unstaged folder2/z &&
 	test_all_match git add --sparse folder2 &&
@@ -1058,7 +1058,7 @@ test_expect_success 'cherry-pick/rebase with conflict outside cone' '
 		# NEEDSWORK: Even though the merge conflict removed the
 		# SKIP_WORKTREE bit from the index entry for folder1/a, we should
 		# warn that this is a problematic add.
-		test_sparse_match test_must_fail git add folder1/a &&
+		test_env GIT_ADVICE=1 test_sparse_match test_must_fail git add folder1/a &&
 		grep "Disable or modify the sparsity rules" sparse-checkout-err &&
 		test_sparse_unstaged folder1/a &&
 		test_all_match git add --sparse folder1/a &&
@@ -1070,7 +1070,7 @@ test_expect_success 'cherry-pick/rebase with conflict outside cone' '
 		# outside of the sparse-checkout cone and does not match an
 		# existing index entry with the SKIP_WORKTREE bit cleared.
 		run_on_all mv folder2/a folder2/z &&
-		test_sparse_match test_must_fail git add folder2 &&
+		test_env GIT_ADVICE=1 test_sparse_match test_must_fail git add folder2 &&
 		grep "Disable or modify the sparsity rules" sparse-checkout-err &&
 		test_sparse_unstaged folder2/z &&
 		test_all_match git add --sparse folder2 &&
@@ -2341,7 +2341,7 @@ test_expect_success 'advice.sparseIndexExpanded' '
 	git -C sparse-index sparse-checkout set deep/deeper1 &&
 	mkdir -p sparse-index/deep/deeper2/deepest &&
 	touch sparse-index/deep/deeper2/deepest/bogus &&
-	git -C sparse-index status 2>err &&
+	GIT_ADVICE=1 git -C sparse-index status 2>err &&
 	grep "The sparse index is expanding to a full index" err
 '
 
diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index 8d90d028504529..43ee72b19bd68c 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -175,7 +175,7 @@ test_expect_success 'tracking count is accurate after orphan check' '
 	git config branch.child.remote . &&
 	git config branch.child.merge refs/heads/main &&
 	git checkout child^ &&
-	git checkout child >stdout &&
+	GIT_ADVICE=1 git checkout child >stdout &&
 	test_cmp expect stdout &&
 
 	git checkout --detach child >stdout &&
@@ -251,15 +251,17 @@ test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not as
 	# Various ways of *not* asking for ellipses
 
 	sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
-	git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	GIT_ADVICE=1 git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
 	check_detached &&
 	test_cmp 1st_detach actual &&
 
-	GIT_PRINT_SHA1_ELLIPSIS="no" git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	GIT_ADVICE=1  GIT_PRINT_SHA1_ELLIPSIS="no" git -c 'core.abbrev=12' \
+		checkout HEAD^ >actual 2>&1 &&
 	check_detached &&
 	test_cmp 2nd_detach actual &&
 
-	GIT_PRINT_SHA1_ELLIPSIS= git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	GIT_ADVICE=1 GIT_PRINT_SHA1_ELLIPSIS= git -c 'core.abbrev=12' \
+		checkout HEAD^ >actual 2>&1 &&
 	check_detached &&
 	test_cmp 3rd_detach actual &&
 
@@ -270,17 +272,17 @@ test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not as
 	check_not_detached &&
 
 	# Make no mention of the env var at all
-	git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	GIT_ADVICE=1 git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
 	check_detached &&
 	test_cmp 1st_detach actual &&
 
 	GIT_PRINT_SHA1_ELLIPSIS='nope' &&
-	git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	GIT_ADVICE=1 git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
 	check_detached &&
 	test_cmp 2nd_detach actual &&
 
 	GIT_PRINT_SHA1_ELLIPSIS=nein &&
-	git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	GIT_ADVICE=1 git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
 	check_detached &&
 	test_cmp 3rd_detach actual &&
 
@@ -333,15 +335,18 @@ test_expect_success 'describe_detached_head does print SHA-1 ellipsis when asked
 	# Various ways of asking for ellipses...
 	# The user can just use any kind of quoting (including none).
 
-	GIT_PRINT_SHA1_ELLIPSIS=yes git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	GIT_ADVICE=1 GIT_PRINT_SHA1_ELLIPSIS=yes git -c 'core.abbrev=12' \
+		checkout HEAD^ >actual 2>&1 &&
 	check_detached &&
 	test_cmp 1st_detach actual &&
 
-	GIT_PRINT_SHA1_ELLIPSIS=Yes git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	GIT_ADVICE=1 GIT_PRINT_SHA1_ELLIPSIS=Yes git -c 'core.abbrev=12' \
+		checkout HEAD^ >actual 2>&1 &&
 	check_detached &&
 	test_cmp 2nd_detach actual &&
 
-	GIT_PRINT_SHA1_ELLIPSIS=YES git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	GIT_ADVICE=1 GIT_PRINT_SHA1_ELLIPSIS=YES git -c 'core.abbrev=12' \
+		checkout HEAD^ >actual 2>&1 &&
 	check_detached &&
 	test_cmp 3rd_detach actual &&
 
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index 2caada3d834ffc..56be88b1620b38 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -103,11 +103,12 @@ test_expect_success 'when arg matches multiple remotes, do not fallback to inter
 test_expect_success 'checkout of branch from multiple remotes fails with advice' '
 	git checkout -B main &&
 	test_might_fail git branch -D foo &&
-	test_must_fail git checkout foo 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git checkout foo 2>stderr &&
 	test_branch main &&
 	status_uno_is_clean &&
 	test_grep "^hint: " stderr &&
-	test_must_fail git -c advice.checkoutAmbiguousRemoteBranchName=false \
+	test_env GIT_ADVICE=1 test_must_fail git \
+		-c advice.checkoutAmbiguousRemoteBranchName=false \
 		checkout foo 2>stderr &&
 	test_branch main &&
 	status_uno_is_clean &&
diff --git a/t/t2060-switch.sh b/t/t2060-switch.sh
index 77b2346291b39c..d84b3accf0e898 100755
--- a/t/t2060-switch.sh
+++ b/t/t2060-switch.sh
@@ -34,13 +34,13 @@ test_expect_success 'switch and detach' '
 '
 
 test_expect_success 'suggestion to detach' '
-	test_must_fail git switch main^{commit} 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git switch main^{commit} 2>stderr &&
 	grep "try again with the --detach option" stderr
 '
 
 test_expect_success 'suggestion to detach is suppressed with advice.suggestDetachingHead=false' '
 	test_config advice.suggestDetachingHead false &&
-	test_must_fail git switch main^{commit} 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git switch main^{commit} 2>stderr &&
 	! grep "try again with the --detach option" stderr
 '
 
diff --git a/t/t2204-add-ignored.sh b/t/t2204-add-ignored.sh
index b7cf1e492c1073..ca46bbd22c7286 100755
--- a/t/t2204-add-ignored.sh
+++ b/t/t2204-add-ignored.sh
@@ -30,7 +30,7 @@ for i in ign dir/ign dir/sub dir/sub/*ign sub/file sub sub/*
 do
 	test_expect_success "complaints for ignored $i" '
 		rm -f .git/index &&
-		test_must_fail git add "$i" 2>err &&
+		test_env GIT_ADVICE=1 test_must_fail git add "$i" 2>err &&
 		git ls-files "$i" >out &&
 		test_must_be_empty out
 	'
@@ -41,7 +41,7 @@ do
 
 	test_expect_success "complaints for ignored $i with unignored file" '
 		rm -f .git/index &&
-		test_must_fail git add "$i" file 2>err &&
+		test_env GIT_ADVICE=1 test_must_fail git add "$i" file 2>err &&
 		git ls-files "$i" >out &&
 		test_must_be_empty out
 	'
@@ -56,7 +56,7 @@ do
 		rm -f .git/index &&
 		(
 			cd dir &&
-			test_must_fail git add "$i" 2>err &&
+			test_env GIT_ADVICE=1 test_must_fail git add "$i" 2>err &&
 			git ls-files "$i" >out &&
 			test_must_be_empty out
 		)
@@ -76,7 +76,7 @@ do
 		rm -f .git/index &&
 		(
 			cd sub &&
-			test_must_fail git add "$i" 2>err &&
+			test_env GIT_ADVICE=1 test_must_fail git add "$i" 2>err &&
 			git ls-files "$i" >out &&
 			test_must_be_empty out
 		)
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index cfc4aeb1798c6d..742002ff41e896 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -436,7 +436,7 @@ test_wt_add_orphan_hint () {
 		git init repo &&
 		(cd repo && test_commit commit) &&
 		git -C repo switch --orphan noref &&
-		test_must_fail git -C repo worktree add $opts foobar/ 2>actual &&
+		test_env GIT_ADVICE=1 test_must_fail git -C repo worktree add $opts foobar/ 2>actual &&
 		! grep "error: unknown switch" actual &&
 		grep "hint: If you meant to create a worktree containing a new unborn branch" actual &&
 		if [ $use_branch -eq 1 ]
@@ -983,7 +983,7 @@ test_dwim_orphan () {
 			fi &&
 			if [ "$outcome" = "infer" ]
 			then
-				git $dashc_args worktree add $args 2>actual &&
+				GIT_ADVICE=1 git $dashc_args worktree add $args 2>actual &&
 				if [ $use_quiet -eq 1 ]
 				then
 					test_must_be_empty actual
@@ -992,7 +992,7 @@ test_dwim_orphan () {
 				fi
 			elif [ "$outcome" = "no_infer" ]
 			then
-				git $dashc_args worktree add $args 2>actual &&
+				GIT_ADVICE=1 git $dashc_args worktree add $args 2>actual &&
 				if [ $use_quiet -eq 1 ]
 				then
 					test_must_be_empty actual
@@ -1001,11 +1001,11 @@ test_dwim_orphan () {
 				fi
 			elif [ "$outcome" = "fetch_error" ]
 			then
-				test_must_fail git $dashc_args worktree add $args 2>actual &&
+				test_env GIT_ADVICE=1 test_must_fail git $dashc_args worktree add $args 2>actual &&
 				grep "$fetch_error_text" actual
 			elif [ "$outcome" = "fatal_orphan_bad_combo" ]
 			then
-				test_must_fail git $dashc_args worktree add $args 2>actual &&
+				test_env GIT_ADVICE=1 test_must_fail git $dashc_args worktree add $args 2>actual &&
 				if [ $use_quiet -eq 1 ]
 				then
 					! grep "$info_text" actual
@@ -1015,7 +1015,7 @@ test_dwim_orphan () {
 				grep "$bad_combo_regex" actual
 			elif [ "$outcome" = "warn_bad_head" ]
 			then
-				test_must_fail git $dashc_args worktree add $args 2>actual &&
+				test_env GIT_ADVICE=1 test_must_fail git $dashc_args worktree add $args 2>actual &&
 				if [ $use_quiet -eq 1 ]
 				then
 					grep "$invalid_ref_regex" actual &&
diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh
index 4dca8d97a772d6..546b6f2f3734ee 100755
--- a/t/t7500-commit-template-squash-signoff.sh
+++ b/t/t7500-commit-template-squash-signoff.sh
@@ -554,7 +554,8 @@ test_expect_success 'commit without staging files fails and displays hints' '
 	git add file &&
 	git commit -m initial &&
 	echo "changes" >>file &&
-	test_must_fail git commit -m update >actual &&
+	test_env GIT_ADVICE=1 test_must_fail \
+		git commit -m update >actual &&
 	test_grep "no changes added to commit (use \"git add\" and/or \"git commit -a\")" actual
 '
 

From 483fcc94355e69e21f76ed4a6d8cd8b885bd7f75 Mon Sep 17 00:00:00 2001
From: Derrick Stolee <derrickstolee@github.com>
Date: Tue, 20 Aug 2024 22:46:19 -0400
Subject: [PATCH 2/7] t3000-4000: add GIT_ADVICE=1 to advice tests

Several tests validate the exact output of stderr, including when the stderr
file should be empty. In advance of modifying the advice system to only
output when stderr is a terminal, force the advice system to output in these
cases.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/t3200-branch.sh               |  4 ++--
 t/t3404-rebase-interactive.sh   |  2 +-
 t/t3501-revert-cherry-pick.sh   |  2 +-
 t/t3507-cherry-pick-conflict.sh |  4 ++--
 t/t3510-cherry-pick-sequence.sh |  6 +++---
 t/t3600-rm.sh                   | 12 ++++++------
 t/t3602-rm-sparse-checkout.sh   | 18 +++++++++---------
 t/t3700-add.sh                  |  6 +++---
 t/t3705-add-sparse-checkout.sh  | 32 ++++++++++++++++----------------
 t/t4150-am.sh                   | 14 +++++++-------
 10 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index ccfa6a720d090c..9ff64fe4f1adec 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1161,7 +1161,7 @@ test_expect_success 'avoid ambiguous track and advise' '
 	hint: different remotes'\'' fetch refspecs map into different
 	hint: tracking namespaces.
 	EOF
-	test_must_fail git branch all1 main 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git branch all1 main 2>actual &&
 	test_cmp expected actual &&
 	test -z "$(git config branch.all1.merge)"
 '
@@ -1699,7 +1699,7 @@ test_expect_success 'errors if given a bad branch name' '
 	hint: See `man git check-ref-format`
 	hint: Disable this message with "git config advice.refSyntax false"
 	EOF
-	test_must_fail git branch foo..bar >actual 2>&1 &&
+	test_env GIT_ADVICE=1 test_must_fail git branch foo..bar >actual 2>&1 &&
 	test_cmp expect actual
 '
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index f92baad1381e66..c31ca807f7bab0 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -2229,7 +2229,7 @@ test_expect_success 'non-merge commands reject merge commits' '
 	EOF
 	(
 		set_replace_editor todo &&
-		test_must_fail git rebase -i HEAD 2>actual
+		test_env GIT_ADVICE=1 test_must_fail git rebase -i HEAD 2>actual
 	) &&
 	cat >expect <<-EOF &&
 	error: ${SQ}pick${SQ} does not accept merge commits
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 411027fb58c7df..3478a8a588faad 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -181,7 +181,7 @@ test_expect_success 'advice from failed revert' '
 	hint: Disable this message with "git config advice.mergeConflict false"
 	EOF
 	test_commit --append --no-tag "double-add dream" dream dream &&
-	test_must_fail git revert HEAD^ 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git revert HEAD^ 2>actual &&
 	test_cmp expected actual
 '
 
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index f3947b400a3a89..5633a10659d5db 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -62,7 +62,7 @@ test_expect_success 'advice from failed cherry-pick' '
 	hint: run "git cherry-pick --abort".
 	hint: Disable this message with "git config advice.mergeConflict false"
 	EOF
-	test_must_fail git cherry-pick picked 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git cherry-pick picked 2>actual &&
 
 	test_cmp expected actual
 '
@@ -77,7 +77,7 @@ test_expect_success 'advice from failed cherry-pick --no-commit' "
 	hint: with 'git add <paths>' or 'git rm <paths>'
 	hint: Disable this message with \"git config advice.mergeConflict false\"
 	EOF
-	test_must_fail git cherry-pick --no-commit picked 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git cherry-pick --no-commit picked 2>actual &&
 
 	test_cmp expected actual
 "
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 7eb52b12edc557..291c5de4f7dc8e 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -231,7 +231,7 @@ test_expect_success 'check advice when we move HEAD by committing' '
 	echo c >foo &&
 	git commit -a &&
 	test_path_is_missing .git/CHERRY_PICK_HEAD &&
-	test_must_fail git cherry-pick --skip 2>advice &&
+	test_env GIT_ADVICE=1 test_must_fail git cherry-pick --skip 2>advice &&
 	test_cmp expect advice
 '
 
@@ -243,7 +243,7 @@ test_expect_success 'selectively advise --skip while launching another sequence'
 	fatal: cherry-pick failed
 	EOF
 	test_must_fail git cherry-pick picked..yetanotherpick &&
-	test_must_fail git cherry-pick picked..yetanotherpick 2>advice &&
+	test_env GIT_ADVICE=1 test_must_fail git cherry-pick picked..yetanotherpick 2>advice &&
 	test_cmp expect advice &&
 	cat >expect <<-EOF &&
 	error: cherry-pick is already in progress
@@ -251,7 +251,7 @@ test_expect_success 'selectively advise --skip while launching another sequence'
 	fatal: cherry-pick failed
 	EOF
 	git reset --merge &&
-	test_must_fail git cherry-pick picked..yetanotherpick 2>advice &&
+	test_env GIT_ADVICE=1 test_must_fail git cherry-pick picked..yetanotherpick 2>advice &&
 	test_cmp expect advice
 '
 
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 31ac31d4bcd3bb..90a30a3a00232b 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -822,7 +822,7 @@ test_expect_success 'rm files with different staged content' '
 	EOF
 	echo content1 >foo.txt &&
 	echo content1 >bar.txt &&
-	test_must_fail git rm foo.txt bar.txt 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git rm foo.txt bar.txt 2>actual &&
 	test_cmp expect actual
 '
 
@@ -847,7 +847,7 @@ test_expect_success 'rm file with local modification' '
 	EOF
 	git commit -m "testing rm 3" &&
 	echo content3 >foo.txt &&
-	test_must_fail git rm foo.txt 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git rm foo.txt 2>actual &&
 	test_cmp expect actual
 '
 
@@ -857,7 +857,7 @@ test_expect_success 'rm file with local modification without hints' '
 	    bar.txt
 	EOF
 	echo content4 >bar.txt &&
-	test_must_fail git -c advice.rmhints=false rm bar.txt 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git -c advice.rmhints=false rm bar.txt 2>actual &&
 	test_cmp expect actual
 '
 
@@ -870,7 +870,7 @@ test_expect_success 'rm file with changes in the index' '
 	git reset --hard &&
 	echo content5 >foo.txt &&
 	git add foo.txt &&
-	test_must_fail git rm foo.txt 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git rm foo.txt 2>actual &&
 	test_cmp expect actual
 '
 
@@ -879,7 +879,7 @@ test_expect_success 'rm file with changes in the index without hints' '
 	error: the following file has changes staged in the index:
 	    foo.txt
 	EOF
-	test_must_fail git -c advice.rmhints=false rm foo.txt 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git -c advice.rmhints=false rm foo.txt 2>actual &&
 	test_cmp expect actual
 '
 
@@ -898,7 +898,7 @@ test_expect_success 'rm files with two different errors' '
 	echo content6 >foo1.txt &&
 	echo content6 >bar1.txt &&
 	git add bar1.txt &&
-	test_must_fail git rm bar1.txt foo1.txt 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git rm bar1.txt foo1.txt 2>actual &&
 	test_cmp expect actual
 '
 
diff --git a/t/t3602-rm-sparse-checkout.sh b/t/t3602-rm-sparse-checkout.sh
index fcdefba48cc76b..c2b197046d423c 100755
--- a/t/t3602-rm-sparse-checkout.sh
+++ b/t/t3602-rm-sparse-checkout.sh
@@ -32,7 +32,7 @@ for opt in "" -f --dry-run
 do
 	test_expect_success "rm${opt:+ $opt} does not remove sparse entries" '
 		git sparse-checkout set --no-cone a &&
-		test_must_fail git rm $opt b 2>stderr &&
+		test_env GIT_ADVICE=1 test_must_fail git rm $opt b 2>stderr &&
 		test_cmp b_error_and_hint stderr &&
 		git ls-files --error-unmatch b
 	'
@@ -72,14 +72,14 @@ test_expect_success 'recursive rm --sparse removes sparse entries' '
 test_expect_success 'rm obeys advice.updateSparsePath' '
 	git reset --hard &&
 	git sparse-checkout set a &&
-	test_must_fail git -c advice.updateSparsePath=false rm b 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git -c advice.updateSparsePath=false rm b 2>stderr &&
 	test_cmp sparse_entry_b_error stderr
 '
 
 test_expect_success 'do not advice about sparse entries when they do not match the pathspec' '
 	git reset --hard &&
 	git sparse-checkout set a &&
-	test_must_fail git rm nonexistent 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git rm nonexistent 2>stderr &&
 	grep "fatal: pathspec .nonexistent. did not match any files" stderr &&
 	! grep -F -f sparse_error_header stderr
 '
@@ -87,7 +87,7 @@ test_expect_success 'do not advice about sparse entries when they do not match t
 test_expect_success 'do not warn about sparse entries when pathspec matches dense entries' '
 	git reset --hard &&
 	git sparse-checkout set a &&
-	git rm "[ba]" 2>stderr &&
+	GIT_ADVICE=1 git rm "[ba]" 2>stderr &&
 	test_must_be_empty stderr &&
 	git ls-files --error-unmatch b &&
 	test_must_fail git ls-files --error-unmatch a
@@ -96,7 +96,7 @@ test_expect_success 'do not warn about sparse entries when pathspec matches dens
 test_expect_success 'do not warn about sparse entries with --ignore-unmatch' '
 	git reset --hard &&
 	git sparse-checkout set a &&
-	git rm --ignore-unmatch b 2>stderr &&
+	GIT_ADVICE=1 git rm --ignore-unmatch b 2>stderr &&
 	test_must_be_empty stderr &&
 	git ls-files --error-unmatch b
 '
@@ -105,9 +105,9 @@ test_expect_success 'refuse to rm a non-skip-worktree path outside sparse cone'
 	git reset --hard &&
 	git sparse-checkout set a &&
 	git update-index --no-skip-worktree b &&
-	test_must_fail git rm b 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git rm b 2>stderr &&
 	test_cmp b_error_and_hint stderr &&
-	git rm --sparse b 2>stderr &&
+	GIT_ADVICE=1 git rm --sparse b 2>stderr &&
 	test_must_be_empty stderr &&
 	test_path_is_missing b
 '
@@ -120,7 +120,7 @@ test_expect_success 'can remove files from non-sparse dir' '
 	test_commit x/y/f &&
 
 	git sparse-checkout set --no-cone w !/x y/ &&
-	git rm w/f.t x/y/f.t 2>stderr &&
+	GIT_ADVICE=1 git rm w/f.t x/y/f.t 2>stderr &&
 	test_must_be_empty stderr
 '
 
@@ -132,7 +132,7 @@ test_expect_success 'refuse to remove non-skip-worktree file from sparse dir' '
 	git sparse-checkout set --no-cone !/x y/ !x/y/z &&
 
 	git update-index --no-skip-worktree x/y/z/f.t &&
-	test_must_fail git rm x/y/z/f.t 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git rm x/y/z/f.t 2>stderr &&
 	echo x/y/z/f.t | cat sparse_error_header - sparse_hint >expect &&
 	test_cmp expect stderr
 '
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 839c904745a286..8042c3bc34aa21 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -34,7 +34,7 @@ test_expect_success 'Test with no pathspecs' '
 	hint: Maybe you wanted to say ${SQ}git add .${SQ}?
 	hint: Disable this message with "git config advice.addEmptyPathspec false"
 	EOF
-	git add 2>actual &&
+	GIT_ADVICE=1 git add 2>actual &&
 	test_cmp expect actual
 '
 
@@ -360,7 +360,7 @@ test_expect_success '"git add" a embedded repository' '
 			git -C $name commit --allow-empty -m $name ||
 				return 1
 		done &&
-		git add . 2>actual &&
+		GIT_ADVICE=1 git add . 2>actual &&
 		cat >expect <<-EOF &&
 		warning: adding embedded git repository: inner1
 		hint: You${SQ}ve added another git repository inside your current repository.
@@ -421,7 +421,7 @@ add 'track-this'
 EOF
 
 test_expect_success 'git add --dry-run --ignore-missing of non-existing file' '
-	test_must_fail git add --dry-run --ignore-missing track-this ignored-file >actual.out 2>actual.err
+	test_env GIT_ADVICE=1 test_must_fail git add --dry-run --ignore-missing track-this ignored-file >actual.out 2>actual.err
 '
 
 test_expect_success 'git add --dry-run --ignore-missing of non-existing file output' '
diff --git a/t/t3705-add-sparse-checkout.sh b/t/t3705-add-sparse-checkout.sh
index 2bade9e804fcfc..c06e803c0e90e2 100755
--- a/t/t3705-add-sparse-checkout.sh
+++ b/t/t3705-add-sparse-checkout.sh
@@ -64,7 +64,7 @@ test_expect_success 'setup' "
 test_expect_success 'git add does not remove sparse entries' '
 	setup_sparse_entry &&
 	rm sparse_entry &&
-	test_must_fail git add sparse_entry 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git add sparse_entry 2>stderr &&
 	test_sparse_entry_unstaged &&
 	test_cmp error_and_hint stderr &&
 	test_sparse_entry_unchanged
@@ -74,7 +74,7 @@ test_expect_success 'git add -A does not remove sparse entries' '
 	setup_sparse_entry &&
 	rm sparse_entry &&
 	setup_gitignore &&
-	git add -A 2>stderr &&
+	GIT_ADVICE=1 git add -A 2>stderr &&
 	test_must_be_empty stderr &&
 	test_sparse_entry_unchanged
 '
@@ -83,7 +83,7 @@ test_expect_success 'git add . does not remove sparse entries' '
 	setup_sparse_entry &&
 	rm sparse_entry &&
 	setup_gitignore &&
-	test_must_fail git add . 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git add . 2>stderr &&
 	test_sparse_entry_unstaged &&
 
 	cat sparse_error_header >expect &&
@@ -99,7 +99,7 @@ do
 	test_expect_success "git add${opt:+ $opt} does not update sparse entries" '
 		setup_sparse_entry &&
 		echo modified >sparse_entry &&
-		test_must_fail git add $opt sparse_entry 2>stderr &&
+		test_env GIT_ADVICE=1 test_must_fail git add $opt sparse_entry 2>stderr &&
 		test_sparse_entry_unstaged &&
 		test_cmp error_and_hint stderr &&
 		test_sparse_entry_unchanged
@@ -110,7 +110,7 @@ test_expect_success 'git add --refresh does not update sparse entries' '
 	setup_sparse_entry &&
 	git ls-files --debug sparse_entry | grep mtime >before &&
 	test-tool chmtime -60 sparse_entry &&
-	test_must_fail git add --refresh sparse_entry 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git add --refresh sparse_entry 2>stderr &&
 	test_sparse_entry_unstaged &&
 	test_cmp error_and_hint stderr &&
 	git ls-files --debug sparse_entry | grep mtime >after &&
@@ -119,7 +119,7 @@ test_expect_success 'git add --refresh does not update sparse entries' '
 
 test_expect_success 'git add --chmod does not update sparse entries' '
 	setup_sparse_entry &&
-	test_must_fail git add --chmod=+x sparse_entry 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git add --chmod=+x sparse_entry 2>stderr &&
 	test_sparse_entry_unstaged &&
 	test_cmp error_and_hint stderr &&
 	test_sparse_entry_unchanged &&
@@ -131,7 +131,7 @@ test_expect_success 'git add --renormalize does not update sparse entries' '
 	test_config core.autocrlf false &&
 	setup_sparse_entry "LINEONE\r\nLINETWO\r\n" &&
 	echo "sparse_entry text=auto" >.gitattributes &&
-	test_must_fail git add --renormalize sparse_entry 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git add --renormalize sparse_entry 2>stderr &&
 	test_sparse_entry_unstaged &&
 	test_cmp error_and_hint stderr &&
 	test_sparse_entry_unchanged
@@ -140,7 +140,7 @@ test_expect_success 'git add --renormalize does not update sparse entries' '
 test_expect_success 'git add --dry-run --ignore-missing warn on sparse path' '
 	setup_sparse_entry &&
 	rm sparse_entry &&
-	test_must_fail git add --dry-run --ignore-missing sparse_entry 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git add --dry-run --ignore-missing sparse_entry 2>stderr &&
 	test_sparse_entry_unstaged &&
 	test_cmp error_and_hint stderr &&
 	test_sparse_entry_unchanged
@@ -148,7 +148,7 @@ test_expect_success 'git add --dry-run --ignore-missing warn on sparse path' '
 
 test_expect_success 'do not advice about sparse entries when they do not match the pathspec' '
 	setup_sparse_entry &&
-	test_must_fail git add nonexistent 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git add nonexistent 2>stderr &&
 	grep "fatal: pathspec .nonexistent. did not match any files" stderr &&
 	! grep -F -f sparse_error_header stderr
 '
@@ -157,7 +157,7 @@ test_expect_success 'do not warn when pathspec matches dense entries' '
 	setup_sparse_entry &&
 	echo modified >sparse_entry &&
 	>dense_entry &&
-	git add "*_entry" 2>stderr &&
+	GIT_ADVICE=1 git add "*_entry" 2>stderr &&
 	test_must_be_empty stderr &&
 	test_sparse_entry_unchanged &&
 	git ls-files --error-unmatch dense_entry
@@ -181,12 +181,12 @@ test_expect_success 'git add fails outside of sparse-checkout definition' '
 	test_sparse_entry_unstaged &&
 
 	# Avoid munging CRLFs to avoid an error message
-	git -c core.autocrlf=input add --sparse sparse_entry 2>stderr &&
+	GIT_ADVICE=1 git -c core.autocrlf=input add --sparse sparse_entry 2>stderr &&
 	test_must_be_empty stderr &&
 	git ls-files --stage >actual &&
 	grep "^100644 .*sparse_entry\$" actual &&
 
-	git add --sparse --chmod=+x sparse_entry 2>stderr &&
+	GIT_ADVICE=1 git add --sparse --chmod=+x sparse_entry 2>stderr &&
 	test_must_be_empty stderr &&
 	git ls-files --stage >actual &&
 	grep "^100755 .*sparse_entry\$" actual &&
@@ -201,7 +201,7 @@ test_expect_success 'git add fails outside of sparse-checkout definition' '
 
 test_expect_success 'add obeys advice.updateSparsePath' '
 	setup_sparse_entry &&
-	test_must_fail git -c advice.updateSparsePath=false add sparse_entry 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git -c advice.updateSparsePath=false add sparse_entry 2>stderr &&
 	test_sparse_entry_unstaged &&
 	test_cmp sparse_entry_error stderr
 
@@ -212,7 +212,7 @@ test_expect_success 'add allows sparse entries with --sparse' '
 	echo modified >sparse_entry &&
 	test_must_fail git add sparse_entry &&
 	test_sparse_entry_unchanged &&
-	git add --sparse sparse_entry 2>stderr &&
+	GIT_ADVICE=1 git add --sparse sparse_entry 2>stderr &&
 	test_must_be_empty stderr
 '
 
@@ -220,7 +220,7 @@ test_expect_success 'can add files from non-sparse dir' '
 	git sparse-checkout set w !/x y/ &&
 	mkdir -p w x/y &&
 	touch w/f x/y/f &&
-	git add w/f x/y/f 2>stderr &&
+	GIT_ADVICE=1 git add w/f x/y/f 2>stderr &&
 	test_must_be_empty stderr
 '
 
@@ -228,7 +228,7 @@ test_expect_success 'refuse to add non-skip-worktree file from sparse dir' '
 	git sparse-checkout set !/x y/ !x/y/z &&
 	mkdir -p x/y/z &&
 	touch x/y/z/f &&
-	test_must_fail git add x/y/z/f 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git add x/y/z/f 2>stderr &&
 	echo x/y/z/f | cat sparse_error_header - sparse_hint >expect &&
 	test_cmp expect stderr
 '
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 5e2b6c80eaedfc..68a62ff330e602 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -678,7 +678,7 @@ test_expect_success 'am -3 -q is quiet' '
 	rm -fr .git/rebase-apply &&
 	git checkout -f lorem2 &&
 	git reset base3way --hard &&
-	git am -3 -q lorem-move.patch >output.out 2>&1 &&
+	GIT_ADVICE=1 git am -3 -q lorem-move.patch >output.out 2>&1 &&
 	test_must_be_empty output.out
 '
 
@@ -921,7 +921,7 @@ test_expect_success 'am -q is quiet' '
 	git reset --hard &&
 	git checkout first &&
 	test_tick &&
-	git am -q <patch1 >output.out 2>&1 &&
+	GIT_ADVICE=1 git am -q <patch1 >output.out 2>&1 &&
 	test_must_be_empty output.out
 '
 
@@ -930,7 +930,7 @@ test_expect_success 'am empty-file does not infloop' '
 	git reset --hard &&
 	touch empty-file &&
 	test_tick &&
-	test_must_fail git am empty-file 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git am empty-file 2>actual &&
 	echo Patch format detection failed. >expected &&
 	test_cmp expected actual
 '
@@ -1180,7 +1180,7 @@ test_expect_success 'apply binary blob in partial clone' '
 
 test_expect_success 'an empty input file is error regardless of --empty option' '
 	test_when_finished "git am --abort || :" &&
-	test_must_fail git am --empty=drop empty.patch 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git am --empty=drop empty.patch 2>actual &&
 	echo "Patch format detection failed." >expected &&
 	test_cmp expected actual
 '
@@ -1188,7 +1188,7 @@ test_expect_success 'an empty input file is error regardless of --empty option'
 test_expect_success 'invalid when passing the --empty option alone' '
 	test_when_finished "git am --abort || :" &&
 	git checkout empty-commit^ &&
-	test_must_fail git am --empty empty-commit.patch 2>err &&
+	test_env GIT_ADVICE=1 test_must_fail git am --empty empty-commit.patch 2>err &&
 	echo "error: invalid value for '\''--empty'\'': '\''empty-commit.patch'\''" >expected &&
 	test_cmp expected err
 '
@@ -1224,7 +1224,7 @@ test_expect_success 'record as an empty commit when meeting e-mail message that
 
 test_expect_success 'skip an empty patch in the middle of an am session' '
 	git checkout empty-commit^ &&
-	test_must_fail git am empty-commit.patch >out 2>err &&
+	test_env GIT_ADVICE=1 test_must_fail git am empty-commit.patch >out 2>err &&
 	grep "Patch is empty." out &&
 	grep "To record the empty patch as an empty commit, run \"git am --allow-empty\"." err &&
 	git am --skip &&
@@ -1236,7 +1236,7 @@ test_expect_success 'skip an empty patch in the middle of an am session' '
 
 test_expect_success 'record an empty patch as an empty commit in the middle of an am session' '
 	git checkout empty-commit^ &&
-	test_must_fail git am empty-commit.patch >out 2>err &&
+	test_env GIT_ADVICE=1 test_must_fail git am empty-commit.patch >out 2>err &&
 	grep "Patch is empty." out &&
 	grep "To record the empty patch as an empty commit, run \"git am --allow-empty\"." err &&
 	git am --allow-empty >output &&

From 970964550ab519c9a8070fada116951bfe04f75d Mon Sep 17 00:00:00 2001
From: Derrick Stolee <derrickstolee@github.com>
Date: Tue, 20 Aug 2024 22:47:16 -0400
Subject: [PATCH 3/7] t5000: add GIT_ADVICE=1 to advice tests

Several tests validate the exact output of stderr, including when the stderr
file should be empty. In advance of modifying the advice system to only
output when stderr is a terminal, force the advice system to output in these
cases.

In particular, lib-https.sh must be updated in order for t5541 to succeed as
it calls test_http_push_nonff.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/lib-httpd.sh             | 2 +-
 t/t5505-remote.sh          | 5 +++--
 t/t5520-pull.sh            | 4 ++--
 t/t5541-http-push-smart.sh | 6 ++++--
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index d83bafeab32d40..b85ce907f05104 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -265,7 +265,7 @@ test_http_push_nonff () {
 		echo "changed" > path2 &&
 		git commit -a -m path2 --amend &&
 
-		test_must_fail git push -v origin >output 2>&1 &&
+		test_env GIT_ADVICE=1 test_must_fail git push -v origin >output 2>&1 &&
 		(
 			cd "$REMOTE_REPO" &&
 			echo "$HEAD" >expect &&
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 08424e878e104c..3e5215add31e88 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -1452,10 +1452,11 @@ test_expect_success 'unqualified <dst> refspec DWIM and advice' '
 			else
 				oid=$(git rev-parse some-tag^{$type})
 			fi &&
-			test_must_fail git push origin $oid:dst 2>err &&
+			test_env GIT_ADVICE=1 test_must_fail git push origin $oid:dst 2>err &&
 			test_grep "error: The destination you" err &&
 			test_grep "hint: Did you mean" err &&
-			test_must_fail git -c advice.pushUnqualifiedRefName=false \
+			test_env GIT_ADVICE=1 test_must_fail git \
+				-c advice.pushUnqualifiedRefName=false \
 				push origin $oid:dst 2>err &&
 			test_grep "error: The destination you" err &&
 			test_grep ! "hint: Did you mean" err ||
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 1098cbd0a19218..c4a309ce4aef9c 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -375,7 +375,7 @@ test_expect_success '--rebase with conflicts shows advice' '
 	echo conflicting >>seq.txt &&
 	test_tick &&
 	git commit -m "Create conflict" seq.txt &&
-	test_must_fail git pull --rebase . seq 2>err >out &&
+	test_env GIT_ADVICE=1 test_must_fail git pull --rebase . seq 2>err >out &&
 	test_grep "Resolve all conflicts manually" err
 '
 
@@ -389,7 +389,7 @@ test_expect_success 'failed --rebase shows advice' '
 	# force checkout because `git reset --hard` will not leave clean `file`
 	git checkout -f -b fails-to-rebase HEAD^ &&
 	test_commit v2-without-cr file "2" file2-lf &&
-	test_must_fail git pull --rebase . diverging 2>err >out &&
+	test_env GIT_ADVICE=1 test_must_fail git pull --rebase . diverging 2>err >out &&
 	test_grep "Resolve all conflicts manually" err
 '
 
diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index 71428f3d5c760a..dfd4c21808f970 100755
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -145,7 +145,7 @@ test_expect_success 'push fails for non-fast-forward refs unmatched by remote he
 
 	# push main too; this ensures there is at least one '"'push'"' command to
 	# the remote helper and triggers interaction with the helper.
-	test_must_fail git push -v origin +main main:niam >output 2>&1'
+	test_env GIT_ADVICE=1 test_must_fail git push -v origin +main main:niam >output 2>&1'
 
 test_expect_success 'push fails for non-fast-forward refs unmatched by remote helper: remote output' '
 	grep "^ + [a-f0-9]*\.\.\.[a-f0-9]* *main -> main (forced update)$" output &&
@@ -477,7 +477,9 @@ test_expect_success 'Non-ASCII branch name can be used with --force-with-lease'
 
 test_expect_success 'colorize errors/hints' '
 	cd "$ROOT_PATH"/test_repo_clone &&
-	test_must_fail git -c color.transport=always -c color.advice=always \
+	test_env GIT_ADVICE=1 test_must_fail git \
+		-c color.transport=always \
+		-c color.advice=always \
 		-c color.push=always \
 		push origin origin/main^:main 2>act &&
 	test_decode_color <act >decoded &&

From 1ce8a8050e13fa5f24dace3ae03ff0e6a5a71aa9 Mon Sep 17 00:00:00 2001
From: Derrick Stolee <derrickstolee@github.com>
Date: Tue, 20 Aug 2024 22:49:00 -0400
Subject: [PATCH 4/7] t6000: add GIT_ADVICE=1 to advice tests

Several tests validate the exact output of stderr, including when the stderr
file should be empty. In advance of modifying the advice system to only
output when stderr is a terminal, force the advice system to output in these
cases.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/t6001-rev-list-graft.sh      |  4 ++--
 t/t6050-replace.sh             |  6 +++---
 t/t6436-merge-overwrite.sh     |  6 +++---
 t/t6437-submodule-merge.sh     | 16 ++++++++--------
 t/t6439-merge-co-error-msgs.sh | 12 ++++++------
 5 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/t/t6001-rev-list-graft.sh b/t/t6001-rev-list-graft.sh
index 3553bbbfe73bd0..e3f196217271df 100755
--- a/t/t6001-rev-list-graft.sh
+++ b/t/t6001-rev-list-graft.sh
@@ -118,10 +118,10 @@ do
 done
 
 test_expect_success 'show advice that grafts are deprecated' '
-	git show HEAD 2>err &&
+	GIT_ADVICE=1 git show HEAD 2>err &&
 	test_grep "git replace" err &&
 	test_config advice.graftFileDeprecated false &&
-	git show HEAD 2>err &&
+	GIT_ADVICE=1 git show HEAD 2>err &&
 	test_grep ! "git replace" err
 '
 
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index c6e9b33e44edf4..fc48cb4b0ad59f 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -489,9 +489,9 @@ test_expect_success '--convert-graft-file' '
 	printf "%s\n%s %s\n\n# comment\n%s\n" \
 		$(git rev-parse HEAD^^ HEAD^ HEAD^^ HEAD^2) \
 		>.git/info/grafts &&
-	git status 2>stderr &&
+	GIT_ADVICE=1 git status 2>stderr &&
 	test_grep "hint:.*grafts is deprecated" stderr &&
-	git replace --convert-graft-file 2>stderr &&
+	GIT_ADVICE=1 git replace --convert-graft-file 2>stderr &&
 	test_grep ! "hint:.*grafts is deprecated" stderr &&
 	test_path_is_missing .git/info/grafts &&
 
@@ -502,7 +502,7 @@ test_expect_success '--convert-graft-file' '
 	: create invalid graft file and verify that it is not deleted &&
 	test_when_finished "rm -f .git/info/grafts" &&
 	echo $EMPTY_BLOB $EMPTY_TREE >.git/info/grafts &&
-	test_must_fail git replace --convert-graft-file 2>err &&
+	test_env GIT_ADVICE=1 test_must_fail git replace --convert-graft-file 2>err &&
 	test_grep "$EMPTY_BLOB $EMPTY_TREE" err &&
 	test_grep "$EMPTY_BLOB $EMPTY_TREE" .git/info/grafts
 '
diff --git a/t/t6436-merge-overwrite.sh b/t/t6436-merge-overwrite.sh
index ccc620477d494b..7c9f5b623f1597 100755
--- a/t/t6436-merge-overwrite.sh
+++ b/t/t6436-merge-overwrite.sh
@@ -104,7 +104,7 @@ test_expect_success 'will not overwrite unstaged changes in renamed file' '
 	cp important other.c &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test_must_fail git merge c1a >out 2>err &&
+		test_env GIT_ADVICE=1 test_must_fail git merge c1a >out 2>err &&
 		test_grep "would be overwritten by merge" err &&
 		test_cmp important other.c &&
 		test_path_is_missing .git/MERGE_HEAD
@@ -140,7 +140,7 @@ test_expect_success 'will not overwrite untracked file in leading path' '
 	rm -rf sub &&
 	cp important sub &&
 	cp important sub2 &&
-	test_must_fail git merge sub 2>out &&
+	test_env GIT_ADVICE=1 test_must_fail git merge sub 2>out &&
 	test_cmp out expect &&
 	test_path_is_missing .git/MERGE_HEAD &&
 	test_cmp important sub &&
@@ -175,7 +175,7 @@ test_expect_success 'will not overwrite untracked file on unborn branch' '
 	git rm -fr . &&
 	git checkout --orphan new &&
 	cp important c0.c &&
-	test_must_fail git merge c0 2>out &&
+	test_env GIT_ADVICE=1 test_must_fail git merge c0 2>out &&
 	test_cmp out expect
 '
 
diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh
index 7a3f1cb27c12b4..9265cebca750d4 100755
--- a/t/t6437-submodule-merge.sh
+++ b/t/t6437-submodule-merge.sh
@@ -113,11 +113,11 @@ test_expect_success 'merging should conflict for non fast-forward' '
 	 git checkout -b test-nonforward-a b &&
 	  if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	  then
-		test_must_fail git merge c 2>actual &&
+		test_env GIT_ADVICE=1 test_must_fail git merge c 2>actual &&
 		sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
 		grep "$sub_expect" actual
 	  else
-		test_must_fail git merge c 2> actual
+		test_env GIT_ADVICE=1 test_must_fail git merge c 2> actual
 	  fi)
 '
 
@@ -154,11 +154,11 @@ test_expect_success 'merging should conflict for non fast-forward (resolution ex
 	  git rev-parse --short sub-d > ../expect) &&
 	  if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	  then
-		test_must_fail git merge c >actual 2>sub-actual &&
+		test_env GIT_ADVICE=1 test_must_fail git merge c >actual 2>sub-actual &&
 		sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
 		grep "$sub_expect" sub-actual
 	  else
-		test_must_fail git merge c 2> actual
+		test_env GIT_ADVICE=1 test_must_fail git merge c 2> actual
 	  fi &&
 	 grep $(cat expect) actual > /dev/null &&
 	 git reset --hard)
@@ -181,11 +181,11 @@ test_expect_success 'merging should fail for ambiguous common parent' '
 	 ) &&
 	 if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	 then
-		test_must_fail git merge c >actual 2>sub-actual &&
+		test_env GIT_ADVICE=1 test_must_fail git merge c >actual 2>sub-actual &&
 		sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" &&
 		grep "$sub_expect" sub-actual
 	 else
-		test_must_fail git merge c 2> actual
+		test_env GIT_ADVICE=1 test_must_fail git merge c 2> actual
 	 fi &&
 	grep $(cat expect1) actual > /dev/null &&
 	grep $(cat expect2) actual > /dev/null &&
@@ -227,7 +227,7 @@ test_expect_success 'merging should fail for changes that are backwards' '
 	git commit -a -m "f" &&
 
 	git checkout -b test-backward e &&
-	test_must_fail git merge f 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git merge f 2>actual &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
     then
 		sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-d)" &&
@@ -535,7 +535,7 @@ test_expect_success 'merging should fail with no merge base' '
 	git checkout -b b init &&
 	git add sub &&
 	git commit -m "b" &&
-	test_must_fail git merge a 2>actual &&
+	test_env GIT_ADVICE=1 test_must_fail git merge a 2>actual &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
     then
 		sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short HEAD^1)" &&
diff --git a/t/t6439-merge-co-error-msgs.sh b/t/t6439-merge-co-error-msgs.sh
index 0cbec57cdabc48..dcc7d45ac757eb 100755
--- a/t/t6439-merge-co-error-msgs.sh
+++ b/t/t6439-merge-co-error-msgs.sh
@@ -40,13 +40,13 @@ Aborting
 EOF
 
 test_expect_success 'untracked files overwritten by merge (fast and non-fast forward)' '
-	test_must_fail git merge branch 2>out &&
+	test_env GIT_ADVICE=1 test_must_fail git merge branch 2>out &&
 	test_cmp out expect &&
 	git commit --allow-empty -m empty &&
 	(
 		GIT_MERGE_VERBOSITY=0 &&
 		export GIT_MERGE_VERBOSITY &&
-		test_must_fail git merge branch 2>out2
+		test_env GIT_ADVICE=1 test_must_fail git merge branch 2>out2
 	) &&
 	echo "Merge with strategy ${GIT_TEST_MERGE_ALGORITHM:-ort} failed." >>expect &&
 	test_cmp out2 expect &&
@@ -69,7 +69,7 @@ test_expect_success 'untracked files or local changes ovewritten by merge' '
 	git add two &&
 	git add three &&
 	git add four &&
-	test_must_fail git merge branch 2>out &&
+	test_env GIT_ADVICE=1 test_must_fail git merge branch 2>out &&
 	test_cmp out expect
 '
 
@@ -91,7 +91,7 @@ test_expect_success 'cannot switch branches because of local changes' '
 	git checkout main &&
 	echo uno >rep/one &&
 	echo dos >rep/two &&
-	test_must_fail git checkout branch 2>out &&
+	test_env GIT_ADVICE=1 test_must_fail git checkout branch 2>out &&
 	test_cmp out expect
 '
 
@@ -105,7 +105,7 @@ EOF
 
 test_expect_success 'not uptodate file porcelain checkout error' '
 	git add rep/one rep/two &&
-	test_must_fail git checkout branch 2>out &&
+	test_env GIT_ADVICE=1 test_must_fail git checkout branch 2>out &&
 	test_cmp out expect
 '
 
@@ -136,7 +136,7 @@ test_expect_success 'not_uptodate_dir porcelain checkout error' '
 	git checkout main &&
 	>rep/untracked-file &&
 	>rep2/untracked-file &&
-	test_must_fail git checkout branch 2>out &&
+	test_env GIT_ADVICE=1 test_must_fail git checkout branch 2>out &&
 	test_cmp out ../expect
 '
 

From ce725bb8991e8f7e61e85e30967f147a6c00a823 Mon Sep 17 00:00:00 2001
From: Derrick Stolee <derrickstolee@github.com>
Date: Tue, 20 Aug 2024 22:49:50 -0400
Subject: [PATCH 5/7] t7000: add GIT_ADVICE=1 to advice tests

Several tests validate the exact output of stderr, including when the stderr
file should be empty. In advance of modifying the advice system to only
output when stderr is a terminal, force the advice system to output in these
cases.

In addition, two more edits were made while in the neighborhood:

 1. In t7002, a redirected stderr was ignored and is now checked as empty.

 2. In t7060 and 7500, the output of "git status" has paranthetical messages
    that appear only when advice is enabled, even though it is sent to stdout.

 3. In t7400, a command was checked for failure with "!" but is now checked
    via test_must_fail.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/t7002-mv-sparse-checkout.sh   | 85 +++++++++++++++++----------------
 t/t7004-tag.sh                  |  2 +-
 t/t7060-wtstatus.sh             | 11 +++--
 t/t7201-co.sh                   |  2 +-
 t/t7400-submodule-basic.sh      |  2 +-
 t/t7402-submodule-rebase.sh     |  3 +-
 t/t7406-submodule-update.sh     |  2 +-
 t/t7512-status-help.sh          |  4 +-
 t/t7520-ignored-hook-warning.sh |  8 ++--
 9 files changed, 61 insertions(+), 58 deletions(-)

diff --git a/t/t7002-mv-sparse-checkout.sh b/t/t7002-mv-sparse-checkout.sh
index 57969ce805a548..3b194bfa2f7bff 100755
--- a/t/t7002-mv-sparse-checkout.sh
+++ b/t/t7002-mv-sparse-checkout.sh
@@ -55,13 +55,13 @@ test_expect_success 'mv refuses to move sparse-to-sparse' '
 	git reset --hard &&
 	git sparse-checkout set --no-cone a &&
 	touch b &&
-	test_must_fail git mv b e 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv b e 2>stderr &&
 	cat sparse_error_header >expect &&
 	echo b >>expect &&
 	echo e >>expect &&
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
-	git mv --sparse b e 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse b e 2>stderr &&
 	test_must_be_empty stderr
 '
 
@@ -72,7 +72,7 @@ test_expect_success 'mv refuses to move sparse-to-sparse, ignores failure' '
 
 	# tracked-to-untracked
 	touch b &&
-	git mv -k b e 2>stderr &&
+	GIT_ADVICE=1 git mv -k b e 2>stderr &&
 	test_path_exists b &&
 	test_path_is_missing e &&
 	cat sparse_error_header >expect &&
@@ -81,7 +81,7 @@ test_expect_success 'mv refuses to move sparse-to-sparse, ignores failure' '
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
 
-	git mv --sparse b e 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse b e 2>stderr &&
 	test_must_be_empty stderr &&
 	test_path_is_missing b &&
 	test_path_exists e &&
@@ -89,7 +89,7 @@ test_expect_success 'mv refuses to move sparse-to-sparse, ignores failure' '
 	# tracked-to-tracked
 	git reset --hard &&
 	touch b &&
-	git mv -k b c 2>stderr &&
+	GIT_ADVICE=1 git mv -k b c 2>stderr &&
 	test_path_exists b &&
 	test_path_is_missing c &&
 	cat sparse_error_header >expect &&
@@ -98,7 +98,7 @@ test_expect_success 'mv refuses to move sparse-to-sparse, ignores failure' '
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
 
-	git mv --sparse b c 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse b c 2>stderr &&
 	test_must_be_empty stderr &&
 	test_path_is_missing b &&
 	test_path_exists c
@@ -110,14 +110,14 @@ test_expect_success 'mv refuses to move non-sparse-to-sparse' '
 	git sparse-checkout set a &&
 
 	# tracked-to-untracked
-	test_must_fail git mv a e 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv a e 2>stderr &&
 	test_path_exists a &&
 	test_path_is_missing e &&
 	cat sparse_error_header >expect &&
 	echo e >>expect &&
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
-	git mv --sparse a e 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse a e 2>stderr &&
 	test_must_be_empty stderr &&
 	test_path_is_missing a &&
 	test_path_exists e &&
@@ -125,14 +125,14 @@ test_expect_success 'mv refuses to move non-sparse-to-sparse' '
 	# tracked-to-tracked
 	rm e &&
 	git reset --hard &&
-	test_must_fail git mv a c 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv a c 2>stderr &&
 	test_path_exists a &&
 	test_path_is_missing c &&
 	cat sparse_error_header >expect &&
 	echo c >>expect &&
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
-	git mv --sparse a c 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse a c 2>stderr &&
 	test_must_be_empty stderr &&
 	test_path_is_missing a &&
 	test_path_exists c
@@ -145,12 +145,12 @@ test_expect_success 'mv refuses to move sparse-to-non-sparse' '
 
 	# tracked-to-untracked
 	touch b &&
-	test_must_fail git mv b e 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv b e 2>stderr &&
 	cat sparse_error_header >expect &&
 	echo b >>expect &&
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
-	git mv --sparse b e 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse b e 2>stderr &&
 	test_must_be_empty stderr
 '
 
@@ -164,7 +164,7 @@ test_expect_success 'recursive mv refuses to move (possible) sparse' '
 	mkdir sub/dir2 &&
 	touch sub/d sub/dir2/e &&
 
-	test_must_fail git mv sub sub2 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv sub sub2 2>stderr &&
 	cat sparse_error_header >expect &&
 	cat >>expect <<-\EOF &&
 	sub/d
@@ -174,7 +174,7 @@ test_expect_success 'recursive mv refuses to move (possible) sparse' '
 	EOF
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
-	git mv --sparse sub sub2 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse sub sub2 2>stderr &&
 	test_must_be_empty stderr &&
 	git commit -m "moved sub to sub2" &&
 	git rev-parse HEAD~1:sub >expect &&
@@ -193,7 +193,7 @@ test_expect_success 'recursive mv refuses to move sparse' '
 	mkdir sub/dir2 &&
 	touch sub/dir2/e &&
 
-	test_must_fail git mv sub sub2 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv sub sub2 2>stderr &&
 	cat sparse_error_header >expect &&
 	cat >>expect <<-\EOF &&
 	sub/dir2/e
@@ -201,7 +201,7 @@ test_expect_success 'recursive mv refuses to move sparse' '
 	EOF
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
-	git mv --sparse sub sub2 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse sub sub2 2>stderr &&
 	test_must_be_empty stderr &&
 	git commit -m "moved sub to sub2" &&
 	git rev-parse HEAD~1:sub >expect &&
@@ -216,8 +216,9 @@ test_expect_success 'can move files to non-sparse dir' '
 	git sparse-checkout set a b c w !/x y/ &&
 	mkdir -p w x/y &&
 
-	git mv a w/new-a 2>stderr &&
-	git mv b x/y/new-b 2>stderr &&
+	GIT_ADVICE=1 git mv a w/new-a 2>stderr &&
+	test_must_be_empty stderr &&
+	GIT_ADVICE=1 git mv b x/y/new-b 2>stderr &&
 	test_must_be_empty stderr
 '
 
@@ -228,7 +229,7 @@ test_expect_success 'refuse to move file to non-skip-worktree sparse path' '
 	git sparse-checkout set a !/x y/ !x/y/z &&
 	mkdir -p x/y/z &&
 
-	test_must_fail git mv a x/y/z/new-a 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv a x/y/z/new-a 2>stderr &&
 	echo x/y/z/new-a | cat sparse_error_header - sparse_hint >expect &&
 	test_cmp expect stderr
 '
@@ -237,7 +238,7 @@ test_expect_success 'refuse to move out-of-cone directory without --sparse' '
 	test_when_finished "cleanup_sparse_checkout" &&
 	setup_sparse_checkout &&
 
-	test_must_fail git mv folder1 sub 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv folder1 sub 2>stderr &&
 	cat sparse_error_header >expect &&
 	echo folder1/file1 >>expect &&
 	cat sparse_hint >>expect &&
@@ -248,7 +249,7 @@ test_expect_success 'can move out-of-cone directory with --sparse' '
 	test_when_finished "cleanup_sparse_checkout" &&
 	setup_sparse_checkout &&
 
-	git mv --sparse folder1 sub 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse folder1 sub 2>stderr &&
 	test_must_be_empty stderr &&
 
 	test_path_is_dir sub/folder1 &&
@@ -259,7 +260,7 @@ test_expect_success 'refuse to move out-of-cone file without --sparse' '
 	test_when_finished "cleanup_sparse_checkout" &&
 	setup_sparse_checkout &&
 
-	test_must_fail git mv folder1/file1 sub 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv folder1/file1 sub 2>stderr &&
 	cat sparse_error_header >expect &&
 	echo folder1/file1 >>expect &&
 	cat sparse_hint >>expect &&
@@ -270,7 +271,7 @@ test_expect_success 'can move out-of-cone file with --sparse' '
 	test_when_finished "cleanup_sparse_checkout" &&
 	setup_sparse_checkout &&
 
-	git mv --sparse folder1/file1 sub 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse folder1/file1 sub 2>stderr &&
 	test_must_be_empty stderr &&
 
 	test_path_is_file sub/file1
@@ -284,7 +285,7 @@ test_expect_success 'refuse to move sparse file to existing destination' '
 	git add folder1 sub/file1 &&
 	git sparse-checkout set --cone sub &&
 
-	test_must_fail git mv --sparse folder1/file1 sub 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv --sparse folder1/file1 sub 2>stderr &&
 	echo "fatal: destination exists, source=folder1/file1, destination=sub/file1" >expect &&
 	test_cmp expect stderr
 '
@@ -298,7 +299,7 @@ test_expect_success 'move sparse file to existing destination with --force and -
 	git add folder1 sub/file1 &&
 	git sparse-checkout set --cone sub &&
 
-	git mv --sparse --force folder1/file1 sub 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse --force folder1/file1 sub 2>stderr &&
 	test_must_be_empty stderr &&
 	echo "overwrite" >expect &&
 	test_cmp expect sub/file1
@@ -308,13 +309,13 @@ test_expect_success 'move clean path from in-cone to out-of-cone' '
 	test_when_finished "cleanup_sparse_checkout" &&
 	setup_sparse_checkout &&
 
-	test_must_fail git mv sub/d folder1 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv sub/d folder1 2>stderr &&
 	cat sparse_error_header >expect &&
 	echo "folder1/d" >>expect &&
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
 
-	git mv --sparse sub/d folder1 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse sub/d folder1 2>stderr &&
 	test_must_be_empty stderr &&
 
 	test_path_is_missing sub/d &&
@@ -330,18 +331,18 @@ test_expect_success 'move clean path from in-cone to out-of-cone overwrite' '
 	echo "sub/file1 overwrite" >sub/file1 &&
 	git add sub/file1 &&
 
-	test_must_fail git mv sub/file1 folder1 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv sub/file1 folder1 2>stderr &&
 	cat sparse_error_header >expect &&
 	echo "folder1/file1" >>expect &&
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
 
-	test_must_fail git mv --sparse sub/file1 folder1 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv --sparse sub/file1 folder1 2>stderr &&
 	echo "fatal: destination exists in the index, source=sub/file1, destination=folder1/file1" \
 	>expect &&
 	test_cmp expect stderr &&
 
-	git mv --sparse -f sub/file1 folder1 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse -f sub/file1 folder1 2>stderr &&
 	test_must_be_empty stderr &&
 
 	test_path_is_missing sub/file1 &&
@@ -366,18 +367,18 @@ test_expect_success 'move clean path from in-cone to out-of-cone file overwrite'
 	echo "sub/file1 overwrite" >sub/file1 &&
 	git add sub/file1 &&
 
-	test_must_fail git mv sub/file1 folder1/file1 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv sub/file1 folder1/file1 2>stderr &&
 	cat sparse_error_header >expect &&
 	echo "folder1/file1" >>expect &&
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
 
-	test_must_fail git mv --sparse sub/file1 folder1/file1 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv --sparse sub/file1 folder1/file1 2>stderr &&
 	echo "fatal: destination exists in the index, source=sub/file1, destination=folder1/file1" \
 	>expect &&
 	test_cmp expect stderr &&
 
-	git mv --sparse -f sub/file1 folder1/file1 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse -f sub/file1 folder1/file1 2>stderr &&
 	test_must_be_empty stderr &&
 
 	test_path_is_missing sub/file1 &&
@@ -403,19 +404,19 @@ test_expect_success 'move directory with one of the files overwrite' '
 	echo test >sub/dir/file1 &&
 	git add sub/dir/file1 &&
 
-	test_must_fail git mv sub/dir folder1 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv sub/dir folder1 2>stderr &&
 	cat sparse_error_header >expect &&
 	echo "folder1/dir/e" >>expect &&
 	echo "folder1/dir/file1" >>expect &&
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
 
-	test_must_fail git mv --sparse sub/dir folder1 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv --sparse sub/dir folder1 2>stderr &&
 	echo "fatal: destination exists in the index, source=sub/dir/file1, destination=folder1/dir/file1" \
 	>expect &&
 	test_cmp expect stderr &&
 
-	git mv --sparse -f sub/dir folder1 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse -f sub/dir folder1 2>stderr &&
 	test_must_be_empty stderr &&
 
 	test_path_is_missing sub/dir/file1 &&
@@ -438,13 +439,13 @@ test_expect_success 'move dirty path from in-cone to out-of-cone' '
 	setup_sparse_checkout &&
 	echo "modified" >>sub/d &&
 
-	test_must_fail git mv sub/d folder1 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv sub/d folder1 2>stderr &&
 	cat sparse_error_header >expect &&
 	echo "folder1/d" >>expect &&
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
 
-	git mv --sparse sub/d folder1 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse sub/d folder1 2>stderr &&
 	cat dirty_error_header >expect &&
 	echo "folder1/d" >>expect &&
 	cat dirty_hint >>expect &&
@@ -462,13 +463,13 @@ test_expect_success 'move dir from in-cone to out-of-cone' '
 	setup_sparse_checkout &&
 	mkdir sub/dir/deep &&
 
-	test_must_fail git mv sub/dir folder1 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv sub/dir folder1 2>stderr &&
 	cat sparse_error_header >expect &&
 	echo "folder1/dir/e" >>expect &&
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
 
-	git mv --sparse sub/dir folder1 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse sub/dir folder1 2>stderr &&
 	test_must_be_empty stderr &&
 
 	test_path_is_missing sub/dir &&
@@ -487,7 +488,7 @@ test_expect_success 'move partially-dirty dir from in-cone to out-of-cone' '
 	echo "modified" >>sub/dir/e2 &&
 	echo "modified" >>sub/dir/e3 &&
 
-	test_must_fail git mv sub/dir folder1 2>stderr &&
+	test_env GIT_ADVICE=1 test_must_fail git mv sub/dir folder1 2>stderr &&
 	cat sparse_error_header >expect &&
 	echo "folder1/dir/e" >>expect &&
 	echo "folder1/dir/e2" >>expect &&
@@ -495,7 +496,7 @@ test_expect_success 'move partially-dirty dir from in-cone to out-of-cone' '
 	cat sparse_hint >>expect &&
 	test_cmp expect stderr &&
 
-	git mv --sparse sub/dir folder1 2>stderr &&
+	GIT_ADVICE=1 git mv --sparse sub/dir folder1 2>stderr &&
 	cat dirty_error_header >expect &&
 	echo "folder1/dir/e2" >>expect &&
 	echo "folder1/dir/e3" >>expect &&
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index b1316e62f46ded..bc216d012cb68d 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1852,7 +1852,7 @@ test_expect_success 'recursive tagging should give advice' '
 	hint: 	git tag -f nested annotated-v4.0^{}
 	hint: Disable this message with "git config advice.nestedTag false"
 	EOF
-	git tag -m nested nested annotated-v4.0 2>actual &&
+	GIT_ADVICE=1 git tag -m nested nested annotated-v4.0 2>actual &&
 	test_cmp expect actual
 '
 
diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
index aaeb4a533440df..8dfb68851567a6 100755
--- a/t/t7060-wtstatus.sh
+++ b/t/t7060-wtstatus.sh
@@ -56,9 +56,10 @@ EOF
 		git rm foo &&
 		git commit -m delete &&
 		test_must_fail git merge main &&
-		test_must_fail git commit --dry-run >../actual &&
+		test_env GIT_ADVICE=1 test_must_fail \
+			git commit --dry-run >../actual &&
 		test_cmp ../expect ../actual &&
-		git status >../actual &&
+		test_env GIT_ADVICE=1 git status >../actual &&
 		test_cmp ../expect ../actual
 	)
 '
@@ -151,7 +152,7 @@ Unmerged paths:
 
 no changes added to commit (use "git add" and/or "git commit -a")
 EOF
-	git status --untracked-files=no >actual &&
+	GIT_ADVICE=1 git status --untracked-files=no >actual &&
 	test_cmp expected actual
 '
 
@@ -185,7 +186,7 @@ Unmerged paths:
 
 no changes added to commit (use "git add" and/or "git commit -a")
 EOF
-	git status --untracked-files=no >actual &&
+	GIT_ADVICE=1 git status --untracked-files=no >actual &&
 	test_cmp expected actual
 '
 
@@ -210,7 +211,7 @@ Unmerged paths:
 
 Untracked files not listed (use -u option to show untracked files)
 EOF
-	git status --untracked-files=no >actual &&
+	GIT_ADVICE=1 git status --untracked-files=no >actual &&
 	test_cmp expected actual &&
 	git reset --hard &&
 	git checkout main
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 2d984eb4c6a167..9ee2374e3d2fb7 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -249,7 +249,7 @@ test_expect_success 'checkout to detach HEAD' '
 	rev=$(git rev-parse --short renamer^) &&
 	git checkout -f renamer &&
 	git clean -f &&
-	git checkout renamer^ 2>messages &&
+	GIT_ADVICE=1 git checkout renamer^ 2>messages &&
 	grep "HEAD is now at $rev" messages &&
 	test_line_count -gt 1 messages &&
 	H=$(git rev-parse --verify HEAD) &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 098d8833b6599d..95e4bacd19ef6e 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -219,7 +219,7 @@ test_expect_success 'submodule add to .gitignored path fails' '
 		echo "*" > .gitignore &&
 		git add --force .gitignore &&
 		git commit -m"Ignore everything" &&
-		! git submodule add "$submodurl" submod >actual 2>&1 &&
+		test_env GIT_ADVICE=1 test_must_fail git submodule add "$submodurl" submod >actual 2>&1 &&
 		test_cmp expect actual
 	)
 '
diff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh
index aa2fdc31d1a672..b155bd6e1c326c 100755
--- a/t/t7402-submodule-rebase.sh
+++ b/t/t7402-submodule-rebase.sh
@@ -116,7 +116,8 @@ test_expect_success 'rebasing submodule that should conflict' '
 	test_tick &&
 	git commit -m fourth &&
 
-	test_must_fail git rebase --onto HEAD^^ HEAD^ HEAD^0 2>actual_output &&
+	test_env GIT_ADVICE=1 test_must_fail git rebase \
+		--onto HEAD^^ HEAD^ HEAD^0 2>actual_output &&
 	git ls-files -s submodule >actual &&
 	(
 		cd submodule &&
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 297c6c3b5cc4b8..560eeea9c99a02 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -206,7 +206,7 @@ test_expect_success 'submodule update should fail due to local changes' '
 	 (cd submodule &&
 	  compare_head
 	 ) &&
-	 test_must_fail git submodule update submodule 2>../actual.raw
+	 test_env GIT_ADVICE=1 test_must_fail git submodule update submodule 2>../actual.raw
 	) &&
 	sed "s/^> //" >expect <<-\EOF &&
 	> error: Your local changes to the following files would be overwritten by checkout:
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index cdd5f2c6979388..de277257d5062f 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -847,7 +847,7 @@ EOF
 test_expect_success 'status shows cherry-pick with invalid oid' '
 	mkdir .git/sequencer &&
 	test_write_lines "pick invalid-oid" >.git/sequencer/todo &&
-	git status --untracked-files=no >actual 2>err &&
+	GIT_ADVICE=1 git status --untracked-files=no >actual 2>err &&
 	git cherry-pick --quit &&
 	test_must_be_empty err &&
 	test_cmp expected actual
@@ -856,7 +856,7 @@ test_expect_success 'status shows cherry-pick with invalid oid' '
 test_expect_success 'status does not show error if .git/sequencer is a file' '
 	test_when_finished "rm .git/sequencer" &&
 	test_write_lines hello >.git/sequencer &&
-	git status --untracked-files=no 2>err &&
+	GIT_ADVICE=1 git status --untracked-files=no 2>err &&
 	test_must_be_empty err
 '
 
diff --git a/t/t7520-ignored-hook-warning.sh b/t/t7520-ignored-hook-warning.sh
index 3b63c34a309de5..21e088894c327e 100755
--- a/t/t7520-ignored-hook-warning.sh
+++ b/t/t7520-ignored-hook-warning.sh
@@ -12,27 +12,27 @@ test_expect_success setup '
 '
 
 test_expect_success 'no warning if hook is not ignored' '
-	git commit --allow-empty -m "more" 2>message &&
+	GIT_ADVICE=1 git commit --allow-empty -m "more" 2>message &&
 	test_grep ! -e "hook was ignored" message
 '
 
 test_expect_success POSIXPERM 'warning if hook is ignored' '
 	test_hook --disable pre-commit &&
-	git commit --allow-empty -m "even more" 2>message &&
+	GIT_ADVICE=1 git commit --allow-empty -m "even more" 2>message &&
 	test_grep -e "hook was ignored" message
 '
 
 test_expect_success POSIXPERM 'no warning if advice.ignoredHook set to false' '
 	test_config advice.ignoredHook false &&
 	test_hook --disable pre-commit &&
-	git commit --allow-empty -m "even more" 2>message &&
+	GIT_ADVICE=1 git commit --allow-empty -m "even more" 2>message &&
 	test_grep ! -e "hook was ignored" message
 '
 
 test_expect_success 'no warning if unset advice.ignoredHook and hook removed' '
 	test_hook --remove pre-commit &&
 	test_unconfig advice.ignoredHook &&
-	git commit --allow-empty -m "even more" 2>message &&
+	GIT_ADVICE=1 git commit --allow-empty -m "even more" 2>message &&
 	test_grep ! -e "hook was ignored" message
 '
 

From 960d1ec11ece90a29da8a909243aeeca0fdc04fb Mon Sep 17 00:00:00 2001
From: Derrick Stolee <derrickstolee@github.com>
Date: Tue, 20 Aug 2024 23:33:38 -0400
Subject: [PATCH 6/7] t7508/12: set GIT_ADVICE=1 across all tests

The output of 'git status' changes depending on the availability of advice,
even though the messages are to stdout. Since this test script is all about
testing the output of 'git status' including the existence (or lack of)
these messages, set the GIT_ADVICE environment globally across the script.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/t7508-status.sh      | 4 ++++
 t/t7512-status-help.sh | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 773383fefb50a9..7158ee57f37c47 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -9,6 +9,10 @@ TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-terminal.sh
 
+# 'git status' output changes depending on the availability of advice,
+# so force its output to enable advice, even though it goes to stdout.
+GIT_ADVICE=1 && export GIT_ADVICE
+
 test_expect_success 'status -h in broken repository' '
 	git config --global advice.statusuoption false &&
 	mkdir broken &&
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index de277257d5062f..1d9676bb3e2c5c 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -17,6 +17,10 @@ TEST_PASSES_SANITIZE_LEAK=true
 
 set_fake_editor
 
+# 'git status' output changes depending on the availability of advice,
+# so force its output to enable advice, even though it goes to stdout.
+GIT_ADVICE=1 && export GIT_ADVICE
+
 test_expect_success 'prepare for conflicts' '
 	git config --global advice.statusuoption false &&
 	test_commit init main.txt init &&

From 25d769903b2ab4a4c454929bf6378751bd366a37 Mon Sep 17 00:00:00 2001
From: Derrick Stolee <derrickstolee@github.com>
Date: Tue, 20 Aug 2024 18:55:37 -0400
Subject: [PATCH 7/7] advice: refuse to output if stderr not TTY

The advice system is intended to help end users around corner cases or other
difficult spots when using the Git tool. As such, they are added without
considering the possibility that they could break scripts or external tools
that execute Git processes and then parse the output.

I will not debate the merit of tools parsing stderr, but instead attempt to
be helpful to tool authors by avoiding these behavior changes across Git
versions.

In b79deeb5544 (advice: add --no-advice global option, 2024-05-03), the
--no-advice option was presented as a way to help tool authors specify that
they do not want any advice messages. As part of this implementation, the
GIT_ADVICE environment variable is given as a way to communicate the desire
for advice (=1) or no advice (=0) and pass that along to all child
processes.

However, both the --no-advice option and the GIT_ADVICE environment variable
require the tool author to change how they interact with Git to gain this
protection.

If Git instead disables the advice system when stderr is not a terminal,
then tool authors benefit immediately.

It is important, though, to let interested users force advice to be enabled,
even when redirecting stderr to a non-terminal file. Be sure to test this by
ensuring GIT_ADVICE=1 forces advice to be written to non-terminals.

The changes leading up to this already set GIT_ADVICE=1 in all other test
scripts that care about the advice being output (or not).

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 Documentation/config/advice.txt |  9 ++++++---
 advice.c                        |  4 +++-
 t/t0018-advice.sh               | 18 +++++++++++++-----
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index 0ba89898207f0c..4946a8aff8d694 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -1,8 +1,11 @@
 advice.*::
 	These variables control various optional help messages designed to
-	aid new users.  When left unconfigured, Git will give the message
-	alongside instructions on how to squelch it.  You can tell Git
-	that you do not need the help message by setting these to `false`:
+	aid new users. These are only output to `stderr` when it is a
+	terminal.
++
+When left unconfigured, Git will give the message alongside instructions
+on how to squelch it.  You can tell Git that you do not need the help
+message by setting these to `false`:
 +
 --
 	addEmbeddedRepo::
diff --git a/advice.c b/advice.c
index 6b879d805c0304..05cf467b6800ba 100644
--- a/advice.c
+++ b/advice.c
@@ -133,7 +133,9 @@ int advice_enabled(enum advice_type type)
 	static int globally_enabled = -1;
 
 	if (globally_enabled < 0)
-		globally_enabled = git_env_bool(GIT_ADVICE_ENVIRONMENT, 1);
+		globally_enabled = git_env_bool(GIT_ADVICE_ENVIRONMENT, -1);
+	if (globally_enabled < 0)
+		globally_enabled = isatty(2);
 	if (!globally_enabled)
 		return 0;
 
diff --git a/t/t0018-advice.sh b/t/t0018-advice.sh
index fac52322a7f673..c63ef070a76d04 100755
--- a/t/t0018-advice.sh
+++ b/t/t0018-advice.sh
@@ -8,7 +8,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-test_expect_success 'advice should be printed when config variable is unset' '
+test_expect_success TTY 'advice should be printed when config variable is unset' '
 	cat >expect <<-\EOF &&
 	hint: This is a piece of advice
 	hint: Disable this message with "git config advice.nestedTag false"
@@ -17,7 +17,7 @@ test_expect_success 'advice should be printed when config variable is unset' '
 	test_cmp expect actual
 '
 
-test_expect_success 'advice should be printed when config variable is set to true' '
+test_expect_success TTY 'advice should be printed when config variable is set to true' '
 	cat >expect <<-\EOF &&
 	hint: This is a piece of advice
 	EOF
@@ -26,13 +26,13 @@ test_expect_success 'advice should be printed when config variable is set to tru
 	test_cmp expect actual
 '
 
-test_expect_success 'advice should not be printed when config variable is set to false' '
+test_expect_success TTY 'advice should not be printed when config variable is set to false' '
 	test_config advice.nestedTag false &&
 	test-tool advise "This is a piece of advice" 2>actual &&
 	test_must_be_empty actual
 '
 
-test_expect_success 'advice should not be printed when --no-advice is used' '
+test_expect_success TTY 'advice should not be printed when --no-advice is used' '
 	q_to_tab >expect <<-\EOF &&
 	On branch trunk
 
@@ -54,7 +54,7 @@ test_expect_success 'advice should not be printed when --no-advice is used' '
 	test_cmp expect actual
 '
 
-test_expect_success 'advice should not be printed when GIT_ADVICE is set to false' '
+test_expect_success TTY 'advice should not be printed when GIT_ADVICE is set to false' '
 	q_to_tab >expect <<-\EOF &&
 	On branch trunk
 
@@ -76,6 +76,8 @@ test_expect_success 'advice should not be printed when GIT_ADVICE is set to fals
 	test_cmp expect actual
 '
 
+# This test also verifies that GIT_ADVICE=1 ignores the requirement
+# that stderr is a terminal.
 test_expect_success 'advice should be printed when GIT_ADVICE is set to true' '
 	q_to_tab >expect <<-\EOF &&
 	On branch trunk
@@ -99,4 +101,10 @@ test_expect_success 'advice should be printed when GIT_ADVICE is set to true' '
 	test_cmp expect actual
 '
 
+test_expect_success 'advice should not be printed when stderr is not a terminal' '
+	test_config advice.nestedTag true &&
+	test-tool advise "This is a piece of advice" 2>actual &&
+	test_must_be_empty actual
+'
+
 test_done