Hi Lucas
On 19/06/2025 23:57, Lucas Seiki Oshiro wrote:
Add the field references.format to the repo-info command. The data
retrieved in this field is the same that currently is obtained by
running `git rev-parse --show-ref-format`.
Mentored-by: Karthik Nayak <karthik.188@xxxxxxxxx>
Mentored-by Patrick Steinhardt <ps@xxxxxx>
Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@xxxxxxxxx>
I've concentrated my comments on the tests as others have commented on
the code itself. In general test bodies should be wrapped in single
quotes rather than double quotes and one should prefer test_cmp() over
test_line_count().
+# Test if a field is correctly returned in both plaintext and json formats.
+#
+# Usage: test_repo_info <label> <init command> <key> <expected value>
+#
+# Arguments:
+# label: the label of the test
+# init command: a command that creates a repository called 'repo', configured
+# accordingly to what is being tested
+# key: the key of the field that is being tested
+# expected value: the value that the field should contain
+test_repo_info () {
+ label=$1
+ init_command=$2
+ key=$3
+ expected_value=$4
+
+ test_expect_success PERLJSON "json: $label" "
This double quote should be a single quote. Unlike the test test title,
the body is passed to eval so there is no need to use double quotes to
expand shell variables. Indeed doing so is counter productive as it
means we pass the result of the variable expansion to eval rather that
the variable name.
+ test_when_finished 'rm -rf repo' &&
+ '$SHELL_PATH' -c '$init_command' &&
There is no need to fork a separate shell process here, you can use
eval "$init_command"
instead.
+ cd repo &&
If you change directory in a test then you must do so in a subshell so
that we return to the original directory when the test finishes. In this
case you're only running a single command in repo so you can use
git -C repo repo-info ...
instead and avoid using "cd" all together.
+ echo '$expected_value' >expect &&
+ git repo-info '$key' >output &&
+ cat output | parse_json >parsed &&
+ grep -F 'row[0].$key' parsed | cut -d ' ' -f 2 >value &&
+ cat value | sed 's/^0$/false/' | sed 's/^1$/true/' >actual &&
sed accepts filenames so there is no need to use "cat" here. It also
accepts multiple expressions so you only need a single command
sed "s/^0\$/false/; s/^1\$/true/" value >actual &&
+ test_cmp expect actual
+ "
Putting all of the above together the test looks like
test_expect_success PERLJSON "json: $label" '
test_when_finished "rm -rf repo" &&
eval "$init_command" &&
echo "$expected_value" >expect &&
git -C repo repo-info "$key" >output &&
cat output | parse_json >parsed &&
grep -F "row[0].$key" parsed | cut -d " " -f 2 >value &&
sed "s/^0\$/false/; s/^1\$/true/" value >actual &&
test_cmp expect actual
'
+ test_expect_success "plaintext: $label" "
+ test_when_finished 'rm -rf repo' &&
+ '$SHELL_PATH' -c '$init_command' &&
+ cd repo &&
+ echo '$expected_value' >expect &&
+ git repo-info --format=plaintext '$key' >output &&
+ cat output | cut -d '=' -f 2 >actual &&
+ test_cmp expect actual
+ "
My comments above apply here as well.
[...]
+test_expect_success 'plaintext: output all default fields' "
The body should be single quoted as it is eval'd
+ git repo-info --format=plaintext >actual &&
+ test_line_count = $DEFAULT_NUMBER_OF_FIELDS actual
test_line_count is a pretty weak assertion. It would be better to use
test_cmp()
git repo-info --format=plaintext >actual &&
sort actual >actual.sorted &&
cat >expect <<-\EOF &&
<EXPECTED OUTPUT>
EOF
test_cmp expect actual.sorted
+"
+
+test_expect_success PERLJSON 'json: output all default fields' "
+ git repo-info --format=json > output &&
+ cat output | parse_json | grep '.*\..*\..*' >actual &&
You do not need to use "cat" here as you can redirect the standard input
when you run parse_json.
parse_json <output | grep ...
+ test_line_count = $DEFAULT_NUMBER_OF_FIELDS actual
This is an even weaker assertion as the number of lines in a json file
is not related to its content. Ideally we should normalize the json
output into a standard format and use test_cmp. I don't know how
practical that is.
Best Wishes
Phillip
