git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/1] [GSoC][PATCH] t3070: refactor test -e command

From
Patrick Steinhardt <ps@pks.im>
Date
Mar 4, 2024, 09:17 UTC
Message-ID
<ZeWRnGSU-eZq8WyE@tanuki>
In-Reply-To
<xmqqzfvjf5tq.fsf@gitster.g>
On Thu, Feb 29, 2024 at 11:06:41AM -0800, Junio C Hamano wrote:
Show 40 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> >> @@ -175,7 +175,7 @@ match() {
> >>         test_expect_success EXPENSIVE_ON_WINDOWS 'cleanup after previous file test' '
> >> -               if test -e .git/created_test_file
> >> +               if test_path_exists .git/created_test_file
> >>                 then
> >>                         git reset &&
> >
> > ... which _do_ use test_path_exists() within a `test_expect_success`
> > block. However, the changes are still undesirable because, as above,
> > this `test -e` is merely part of the normal control-flow; it's not
> > acting as an assertion, thus test_path_exists() -- which is an
> > assertion -- is not correct.
> >
> > Unfortunately, none of the uses of`test -e` in t3070 are being used as
> > assertions worthy of replacement with test_path_exists(), thus this
> > isn't a good script in which to make such changes.
> 
> It seems that there is a recurring confusion among mentorship
> program applicants that use test_path_* helpers as their practice
> material.  Perhaps the source of the information that suggests it as
> a microproject is poorly phrased and needs to be rewritten to avoid
> misleading them.
> 
> I found one at https://git.github.io/Outreachy-23-Microprojects/,
> which can be one source of such confusion:
> 
>     Find one test script that verifies the presence/absence of
>     files/directories with ‘test -(e|f|d|…)’ and replace them
>     with the appropriate test_path_is_file, test_path_is_dir,
>     etc. helper functions.
> 
> but there may be others.
> 
> This task specification does not differenciate "test -[efdx]" used
> as a conditional of a control flow statement (which should never be
> replaced by test_path_* helpers) and those used to directly fail the
> &&-chain in test_expect_success with their exit status (which is the
> target that test_path_* helpers are meant to improve).

Good point. I've sent a patch in reply to your message that hopefully clarifies this a bit. Thanks!

Patrick
Previous: Junio C HamanoNext: shejialuo
Message 8 of 26 in “microproject: Use test_path_is_* functions in test scripts”
  1. shejialuoFeb 29, 2024
  2. 1/1 [GSoC][PATCH] t3070: refactor test -e commandshejialuo, Feb 29, 2024
  3. Eric SunshineFeb 29, 2024
  4. Junio C HamanoFeb 29, 2024
  5. SoC 2024: clarify `test_path_is_*` conversion microprojectPatrick Steinhardt, Mar 4, 2024
  6. Christian CouderMar 4, 2024
  7. Junio C HamanoMar 4, 2024
  8. Patrick SteinhardtMar 4, 2024
  9. shejialuoMar 1, 2024
  10. 0/1 [GSoC][PATCH] t9117: prefer test_path_* helper functionsshejialuo, Mar 1, 2024
  11. 1/1 t9117: prefer test_path_* helper functionsshejialuo, Mar 1, 2024
  12. Eric SunshineMar 1, 2024
  13. shejialuoMar 1, 2024
  14. Junio C HamanoMar 1, 2024
  15. shejialuoMar 1, 2024
  16. 0/1 t9117: prefer test_path_* helper functionsshejialuo, Mar 1, 2024
  17. 1/1 [PATCH] t9117: prefer test_path_* helper functionsshejialuo, Mar 1, 2024
  18. Patrick SteinhardtMar 4, 2024
  19. 0/1 Change commit messageshejialuo, Mar 4, 2024
  20. 1/1 [PATCH] t9117: prefer test_path_* helper functionsshejialuo, Mar 4, 2024
  21. Patrick SteinhardtMar 4, 2024
  22. shejialuoMar 4, 2024
  23. Junio C HamanoMar 4, 2024
  24. Junio C HamanoMar 4, 2024
  25. shejialuoMar 5, 2024
  26. Junio C HamanoMar 4, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.