Skip to contentSkip to conversation

t9803-git-p4-shell-metachars.sh: update to use test_path_* functions

PATCH Superseded Sanchit Jindal via GitGitGadget 4 participants 9 messages Mar 21, 2024

lore
Share & tools
lore
#0 lore raw

cover t9803-git-p4-shell-metachars.sh: update to use test_path_* functions

Correct Typo in Branch name

Sanchit Jindal (1):
t9803: update commit messages and description

sanchit1053 (1):
t9803-git-p4-shell-metachars.sh: update to use test_path_* functions

 t/t9803-git-p4-shell-metachars.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

base-commit: 3bd955d26919e149552f34aacf8a4e6368c26cec
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1691%2Fsanchit1053%2Fsj%2Ft9803_use_path_helper_fn-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1691/sanchit1053/sj/t9803_use_path_helper_fn-v1
Pull-Request: https://github.com/git/git/pull/1691

Signature · 1 line
gitgitgadget
#1 lore raw

PATCH 1/2 t9803-git-p4-shell-metachars.sh: update to use test_path_* functions

From: sanchit1053 <[email protected]>

---
 t/t9803-git-p4-shell-metachars.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
t/t9803-git-p4-shell-metachars.sh +6 -6
index 2913277013d..4905ed2ae9e 100755
@@ -33,8 +33,8 @@ test_expect_success 'shell metachars in filenames' '
 	(
 		cd "$cli" &&
 		p4 sync ... &&
-		test -e "file with spaces" &&
-		test -e "foo\$bar"
+		test_path_exists "file with spaces" &&
+		test_path_exists "foo\$bar"
 	)
 '
 
@@ -52,8 +52,8 @@ test_expect_success 'deleting with shell metachars' '
 	(
 		cd "$cli" &&
 		p4 sync ... &&
-		test ! -e "file with spaces" &&
-		test ! -e foo\$bar
+		test_path_is_missing "file with spaces" &&
+		test_path_is_missing foo\$bar
 	)
 '
 
@@ -100,8 +100,8 @@ test_expect_success 'branch with shell char' '
 		git p4 clone --dest=. --detect-branches //depot@all &&
 		git log --all --graph --decorate --stat &&
 		git reset --hard p4/depot/branch\$3 &&
-		test -f shell_char_branch_file &&
-		test -f f1
+		test_path_is_file shell_char_branch_file &&
+		test_path_is_file f1
 	)
 '
 
Signature · 1 line
gitgitgadget
#2 lore raw

PATCH 2/2 t9803: update commit messages and description

In reply to Sanchit Jindal via GitGitGadget

From: Sanchit Jindal <[email protected]>

replacing test -e with test_path_exists,
test ! -e with test_path_is_missing
test -f with test_path_is_file
These helper functions will run the test command with the corresponding flags and will echo a message if the assert fails. This will provide better debugging logs for test, instead of the previous method which provided no message

Signature · 1 line
gitgitgadget
#3 lore raw

Re: [PATCH 2/2] t9803: update commit messages and description

Quoted 11 lines · Sanchit Jindal via GitGitGadget

From: Sanchit Jindal <[email protected]>

replacing test -e with test_path_exists,
test ! -e with test_path_is_missing
test -f with test_path_is_file
These helper functions will run the test command with the corresponding flags and will echo a message if the assert fails. This will provide better debugging logs for test, instead of the previous method which provided no message

When rerolling a series to address reviewer comments, you will want the fixes applied directly to the patches about which the reviewers commented. The way to do this is to use git rebase -i to adjust the patches as needed. In the case of this simple series, you just want to "squash" patches [1/2] and [2/2] into a single patch using the git rebase -i "squash" command, and adjust the commit message of the squashed patch appropriately. Finally, to resubmit it via GitGitGadget, force-push the revised series to GitGitGadget (using git push --force <whatever> <whatever>), and tell GitGitGadget to "/submit".

Regarding the commit message, first explain the problem the patch is solving, and then explain how the patch solves it. Thus, start by explaining that test doesn't provide any diagnostic information when it fails, which isn't helpful to test authors. Then explain that the patch replaces test with the test_path_* functions which do provide useful diagnostic information.

The From: and Signed-off-by: lines look good in this reroll.

#4 lore raw

Re: [PATCH 2/2] t9803: update commit messages and description

Quoted 26 lines · Eric Sunshine
Quoted 11 lines · Sanchit Jindal via GitGitGadget

From: Sanchit Jindal <[email protected]>

replacing test -e with test_path_exists,
test ! -e with test_path_is_missing
test -f with test_path_is_file
These helper functions will run the test command with the corresponding flags and will echo a message if the assert fails. This will provide better debugging logs for test, instead of the previous method which provided no message

When rerolling a series to address reviewer comments, you will want the fixes applied directly to the patches about which the reviewers commented. The way to do this is to use git rebase -i to adjust the patches as needed. In the case of this simple series, you just want to "squash" patches [1/2] and [2/2] into a single patch using the git rebase -i "squash" command, and adjust the commit message of the squashed patch appropriately. Finally, to resubmit it via GitGitGadget, force-push the revised series to GitGitGadget (using git push --force <whatever> <whatever>), and tell GitGitGadget to "/submit".

Quoted 8 lines

Regarding the commit message, first explain the problem the patch is solving, and then explain how the patch solves it. Thus, start by explaining that test doesn't provide any diagnostic information when it fails, which isn't helpful to test authors. Then explain that the patch replaces test with the test_path_* functions which do provide useful diagnostic information.

The From: and Signed-off-by: lines look good in this reroll.

Thank You for the review

I renamed the branch using github as it had a typo which made the earlier patch to be erased.I am extremely sorry about this.

I have created another patch with a more descriptive commit message, and after squashing it with the earlier commit. I hope it is satisfactory

I wanted to ask, Is it possible to send the patches created using git format-patch manually using gmail, Or the default headers applied by send-email required.

Regards,
Sanchit Jindal

#6 lore raw

v2 cover t9803-git-p4-shell-metachars.sh: update to use test_path_* functions

In reply to Sanchit Jindal via GitGitGadget

Correct Typo in Branch name

Sanchit Jindal (1):
t9803: update commit messages and description

sanchit1053 (1):
t9803-git-p4-shell-metachars.sh: update to use test_path_* functions

 t/t9803-git-p4-shell-metachars.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

base-commit: 3bd955d26919e149552f34aacf8a4e6368c26cec
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1691%2Fsanchit1053%2Fsj%2Ft9803_use_path_helper_fn-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1691/sanchit1053/sj/t9803_use_path_helper_fn-v2
Pull-Request: https://github.com/git/git/pull/1691

Range-diff vs v1:

 1:  b541ce7a49c = 1:  b541ce7a49c t9803-git-p4-shell-metachars.sh: update to use test_path_* functions
 2:  b8d0620d410 < -:  ----------- t9803: update commit messages and description
 -:  ----------- > 2:  e25b5f928a9 t9803: update commit messages and description
Signature · 1 line
gitgitgadget
#7 lore raw

PATCH v2 1/2 t9803-git-p4-shell-metachars.sh: update to use test_path_* functions

From: sanchit1053 <[email protected]>

---
 t/t9803-git-p4-shell-metachars.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
t/t9803-git-p4-shell-metachars.sh +6 -6
index 2913277013d..4905ed2ae9e 100755
@@ -33,8 +33,8 @@ test_expect_success 'shell metachars in filenames' '
 	(
 		cd "$cli" &&
 		p4 sync ... &&
-		test -e "file with spaces" &&
-		test -e "foo\$bar"
+		test_path_exists "file with spaces" &&
+		test_path_exists "foo\$bar"
 	)
 '
 
@@ -52,8 +52,8 @@ test_expect_success 'deleting with shell metachars' '
 	(
 		cd "$cli" &&
 		p4 sync ... &&
-		test ! -e "file with spaces" &&
-		test ! -e foo\$bar
+		test_path_is_missing "file with spaces" &&
+		test_path_is_missing foo\$bar
 	)
 '
 
@@ -100,8 +100,8 @@ test_expect_success 'branch with shell char' '
 		git p4 clone --dest=. --detect-branches //depot@all &&
 		git log --all --graph --decorate --stat &&
 		git reset --hard p4/depot/branch\$3 &&
-		test -f shell_char_branch_file &&
-		test -f f1
+		test_path_is_file shell_char_branch_file &&
+		test_path_is_file f1
 	)
 '
 
Signature · 1 line
gitgitgadget
#8 lore raw

PATCH v2 2/2 t9803: update commit messages and description

In reply to Sanchit Jindal via GitGitGadget

From: Sanchit Jindal <[email protected]>

Using the test function directly does not provide any diagnostic information to the user, in case of a failure.

This patch replace the test function with the helper functions defined that will call the corresponding test function and log an error message if the assert fails

Making the replacements
- test -e -> test_path_exists,
- test ! -e -> test_path_is_missing
- test -f -> test_path_is_file

Signature · 1 line
gitgitgadget
#5 lore raw

Re: [PATCH 2/2] t9803: update commit messages and description

In reply to Sanchit Jindal
Quoted 2 lines · Sanchit Jindal

I have created another patch with a more descriptive commit message, and after squashing it with the earlier commit. I hope it is satisfactory

Hopefully. What GGG sent with v2 label still was a 2-patch series, whose [2/2] patch was an empty patch, though.

Quoted 3 lines

I wanted to ask, Is it possible to send the patches created using git format-patch manually using gmail, Or the default headers applied by send-email required.

I do not know where you are using gmail from (I know that Android version has no way to send text-only e-mail for example), but when cutting and pasting the output from format-patch, you should make sure

* Remove "From <commit object name> Mon Sep 17 00:00:00 2001" marker line that signals the beginning of each patch message. It is a mistake to leave this line in the body of your message.

* Remove "Date: " header. Do not leave this line in the body of your message.

* Remove "Subject: " header and move its contents to your MUA's Subject entry field.

* Remove "From: " header, and arrange that your MUA puts the same "Sanchit Jindal <[email protected]> on the From header. If your MUA is not cooperating, you can leave that line in the body of the message

* Your cut&paste does not corrupt whitespaces, like squashing two consecutive spaces into one, removing the leading whitespaces, turning a tab into a run of spaces, folding a line at a whitespace in the middle of the message, etc.

* Your MUA does not turn your text message into HTML gunk.

As long as you do the above carefully, you should be OK. "git format-patch --help" has a section on MUA specific hints, which also might be helpful.

Thanks.