[GSOC][RFC] microproject: use test_path_is_* functions in test scripts

[GSOC][RFC] microproject: use test_path_is_* functions in test scripts

[Vincenzo MEZZELA]

Hello everyone,

I'm Vincenzo, a Master's degree student in Computer Engineering and 
Cybersecurity.

As I'm approaching the end of my academic journey, I'm eager to 
contribute to the Git project
and the GSoC represents a good opportunity to do so. Upon exploring the 
online documentation
of git for the application to the GSoC I'm keen to begin the required 
microproject.

Among the microproject proposed here 
https://git.github.io/SoC-2024-Microprojects/ , I would like to
work on replacing 'test -(e|f|g|...)' with test_path_is* .

Can you confirm if this task has already been taken by someone else?


Approach:

As far as I understood, The work consists in replacing the shell 'test' 
command in the test
script under 't/' directory with the ones present in the 
t/test_lib_functions.sh as follows:

- test -f --> test_path_is_file

- test -d --> test_path_is_dir

- test -e --> test_path_exists


To approximately measure the number of required replacement, I run the 
following commands from the
't/' directory (branch master):

> # Files  that requires a replacement
 > git grep -r 'test -[efdx]' 2>/dev/null| awk '{print $1}' | uniq -c | 
sort -n -r
>>    190 t7301-clean-interactive.sh:
>>     147 t7300-clean.sh:
>>      21 t2004-checkout-cache-temp.sh:
>>      17 t2401-worktree-prune.sh:
>>      16 t2003-checkout-cache-mkdir.sh:
>>      14 t0601-reffiles-pack-refs.sh:
>>      13 t4200-rerere.sh:
>>      12 t9146-git-svn-empty-dirs.sh:
>>      12 t7603-merge-reduce-heads.sh:
>>      12 lib-submodule-update.sh:
>>    ...
 >>
 > # Number of replacements
 > git grep -r 'test -[efdx]' 2>/dev/null| awk '{print $1}' | uniq -c | 
sort -n -r | awk '{sum += $1} END {print sum}'
 >> 853
 > # Number of files that requires a patch
 > git grep -r 'test -[efdx]' 2>/dev/null| awk '{print $1}' | uniq -c | 
wc -l
 >> 169

Although the replacement work might not be difficult, it spans over many 
different test files.
Do you want me to submit a patch for each of them as part of the 
microproject?
If not, How many patches do you expect me to submit?

Thanks,
Vincenzo

Re: [GSOC][RFC] microproject: use test_path_is_* functions in test scripts

[Christian Couder]

Hi,

On Tue, Feb 13, 2024 at 5:10 PM Vincenzo MEZZELA
<vincenzo.mezzela@gmail.com> wrote:
>
> Hello everyone,
>
> I'm Vincenzo, a Master's degree student in Computer Engineering and
> Cybersecurity.
>
> As I'm approaching the end of my academic journey, I'm eager to
> contribute to the Git project
> and the GSoC represents a good opportunity to do so. Upon exploring the
> online documentation
> of git for the application to the GSoC I'm keen to begin the required
> microproject.

Thanks for your interest in working on Git!

> Among the microproject proposed here
> https://git.github.io/SoC-2024-Microprojects/ , I would like to
> work on replacing 'test -(e|f|g|...)' with test_path_is* .
>
> Can you confirm if this task has already been taken by someone else?

This is a generic microproject so even if someone else is working on
this microproject, you can also work on it as long as you don't work
on the same file as that person.

> Approach:
>
> As far as I understood, The work consists in replacing the shell 'test'
> command in the test
> script under 't/' directory with the ones present in the
> t/test_lib_functions.sh as follows:
>
> - test -f --> test_path_is_file
>
> - test -d --> test_path_is_dir
>
> - test -e --> test_path_exists

Note that you only need to do that replacement in one test script
under the 't/' directory.

> To approximately measure the number of required replacement, I run the
> following commands from the
> 't/' directory (branch master):
>
> > # Files  that requires a replacement
>  > git grep -r 'test -[efdx]' 2>/dev/null| awk '{print $1}' | uniq -c |
> sort -n -r
> >>    190 t7301-clean-interactive.sh:
> >>     147 t7300-clean.sh:
> >>      21 t2004-checkout-cache-temp.sh:
> >>      17 t2401-worktree-prune.sh:
> >>      16 t2003-checkout-cache-mkdir.sh:
> >>      14 t0601-reffiles-pack-refs.sh:
> >>      13 t4200-rerere.sh:
> >>      12 t9146-git-svn-empty-dirs.sh:
> >>      12 t7603-merge-reduce-heads.sh:
> >>      12 lib-submodule-update.sh:
> >>    ...
>  >>
>  > # Number of replacements
>  > git grep -r 'test -[efdx]' 2>/dev/null| awk '{print $1}' | uniq -c |
> sort -n -r | awk '{sum += $1} END {print sum}'
>  >> 853
>  > # Number of files that requires a patch
>  > git grep -r 'test -[efdx]' 2>/dev/null| awk '{print $1}' | uniq -c |
> wc -l
>  >> 169
>
> Although the replacement work might not be difficult, it spans over many
> different test files.
> Do you want me to submit a patch for each of them as part of the
> microproject?
> If not, How many patches do you expect me to submit?

We would like a single patch that performs the necessary replacements
in a single file.

For reference in https://git.github.io/General-Microproject-Information/ we say:

"Change only a few files"

and

"This means that for a microproject that consists in refactoring or
rewriting a small amount of code, your patch should change only ONE
file, or perhaps 2 files if they are closely related, like “foo.c” and
“foo.h”."

Thanks,
Christian.

Re: [GSOC][RFC] microproject: use test_path_is_* functions in test scripts

[Eric Sunshine]

On Tue, Feb 13, 2024 at 12:36 PM Christian Couder
<christian.couder@gmail.com> wrote:
> On Tue, Feb 13, 2024 at 5:10 PM Vincenzo MEZZELA
> <vincenzo.mezzela@gmail.com> wrote:
> > Can you confirm if this task has already been taken by someone else?
>
> This is a generic microproject so even if someone else is working on
> this microproject, you can also work on it as long as you don't work
> on the same file as that person.
>
> > >>      12 t9146-git-svn-empty-dirs.sh:

This particular test script is being tackled by someone else on the
list presently, so probably best to choose one of the others.

Re: [GSOC][RFC] microproject: use test_path_is_* functions in test scripts

[Junio C Hamano]

Vincenzo MEZZELA <vincenzo.mezzela@gmail.com> writes:

> Approach:
>
> As far as I understood, The work consists in replacing the shell
> 'test' command in the test
> script under 't/' directory with the ones present in the
> t/test_lib_functions.sh as follows:
>
> - test -f --> test_path_is_file
>
> - test -d --> test_path_is_dir
>
> - test -e --> test_path_exists

One thing to note is that you'd need to make sure if "test -e" for
example originally written really means "we want to see something
there and it does not matter if it is a file or a directory" while
turning it into test_path_exists.  The operations that such a test
follows may be expected to create a file and never a directory, in
which case the condition the original code is testing may need to
be corrected first to expect a more specific type (e.g. "test -f").
The same comment applies for the other two.

Some tests check with "! test -f <path>", which often would want to
be turned into "test_path_is_missing", but you'd need to make sure
that is what the original test really meant to do.

A microproject is not about "doing the real work to help the
project".  It is a practice session to come up with a set of small
changes and explain them well, to send the result to the list to get
reviewed, to respond to reviews and possibly send updates, and to
repeat the cycle until completion.  So most likely you'd pick a
single file or two and do the above conversion, while leaving the
others as practice material for other GSoC participants.

Enjoy.

Re: [GSOC][RFC] microproject: use test_path_is_* functions in test scripts

[vincenzo mezzela]

> One thing to note is that you'd need to make sure if "test -e" for
> example originally written really means "we want to see something
> there and it does not matter if it is a file or a directory" while
> turning it into test_path_exists.  The operations that such a test
> follows may be expected to create a file and never a directory, in
> which case the condition the original code is testing may need to
> be corrected first to expect a more specific type (e.g. "test -f").
> The same comment applies for the other two.

Thanks for this clarification!

> Some tests check with "! test -f <path>", which often would want to
> be turned into "test_path_is_missing", but you'd need to make sure
> that is what the original test really meant to do.

I'd have changed them into:
-  ! test -f <path> --> ! test_path_is_file <path>
-  ! test -e <path> --> test_path_is_missing <path>
Since '! test -f <path>' and 'test_path_is_missing <path>' can return
different values.

But as you said, I'll check what they are really meant to do and
modify them accordingly :)

>
> A microproject is not about "doing the real work to help the
> project".  It is a practice session to come up with a set of small
> changes and explain them well, to send the result to the list to get
> reviewed, to respond to reviews and possibly send updates, and to
> repeat the cycle until completion.  So most likely you'd pick a
> single file or two and do the above conversion, while leaving the
> others as practice material for other GSoC participants.
>
> Enjoy.
>

Thanks for this clarification too. :)

Vincenzo