Subject: Re: [RFC PATCH v2 4/6] test-tool run-command testsuite: support unit tests

From: [email-masked]
Date: Mon, 5 Feb 2024 16:16:35 +0000

Hi Josh

On 03/02/2024 00:50, Josh Steadmon wrote:
> Teach the testsuite runner in `test-tool run-command testsuite` how to
> run unit tests: if TEST_SHELL_PATH is not set, assume that we're running
> the programs directly from CWD, rather than defaulting to "sh" as an
> interpreter.

Judging from the last patch in this series it seems likely that we'll 
want to run unit tests and integration tests parallel. In which case it 
might be better to look at the filename extension to decide whether to 
sh as an interpreter so that we can avoid having to use a wrapper 
script. Then

     cd t
     helper/test-tool run-command testsuite 't[0-9]*.sh' 'unit-tests/bin/*'

would run the integration tests via "sh" and the unit-tests directly. 
We'd need to figure out how to look for tests in both directories as 
well though...

Best Wishes

Phillip

> With this change, you can now use test-tool to run the unit tests:
> $ make
> $ cd t/unit-tests/bin
> $ ../../helper/test-tool run-command testsuite
> 
> This should be helpful on Windows to allow running tests without
> requiring Perl (for `prove`), as discussed in [1] and [2].
> 
> This again breaks backwards compatibility, as it is now required to set
> TEST_SHELL_PATH properly for executing shell scripts, but again, as
> noted in [2], there are no longer any such invocations in our codebase.
> 
> [1] https://lore.kernel.org/git/[email-masked]/
> [2] https://lore.kernel.org/git/[email-masked]/
> 
> Signed-off-by: Josh Steadmon <[email-masked]>
> ---
>   t/helper/test-run-command.c | 18 +++++++++++++++---
>   1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
> index e6bd792274..a0b8dc6fd7 100644
> --- a/t/helper/test-run-command.c
> +++ b/t/helper/test-run-command.c
> @@ -158,6 +158,8 @@ static int testsuite(int argc, const char **argv)
>   		.task_finished = test_finished,
>   		.data = &suite,
>   	};
> +	struct strbuf progpath = STRBUF_INIT;
> +	size_t path_prefix_len;
>   
>   	argc = parse_options(argc, argv, NULL, options,
>   			testsuite_usage, PARSE_OPT_STOP_AT_NON_OPTION);
> @@ -165,9 +167,14 @@ static int testsuite(int argc, const char **argv)
>   	if (max_jobs <= 0)
>   		max_jobs = online_cpus();
>   
> +	/*
> +	 * If we run without a shell, we have to provide the relative path to
> +	 * the executables.
> +	 */
>   	suite.shell_path = getenv("TEST_SHELL_PATH");
>   	if (!suite.shell_path)
> -		suite.shell_path = "sh";
> +		strbuf_addstr(&progpath, "./");
> +	path_prefix_len = progpath.len;
>   
>   	dir = opendir(".");
>   	if (!dir)
> @@ -180,13 +187,17 @@ static int testsuite(int argc, const char **argv)
>   
>   		/* No pattern: match all */
>   		if (!argc) {
> -			string_list_append(&suite.tests, p);
> +			strbuf_setlen(&progpath, path_prefix_len);
> +			strbuf_addstr(&progpath, p);
> +			string_list_append(&suite.tests, progpath.buf);
>   			continue;
>   		}
>   
>   		for (i = 0; i < argc; i++)
>   			if (!wildmatch(argv[i], p, 0)) {
> -				string_list_append(&suite.tests, p);
> +				strbuf_setlen(&progpath, path_prefix_len);
> +				strbuf_addstr(&progpath, p);
> +				string_list_append(&suite.tests, progpath.buf);
>   				break;
>   			}
>   	}
> @@ -213,6 +224,7 @@ static int testsuite(int argc, const char **argv)
>   
>   	string_list_clear(&suite.tests, 0);
>   	string_list_clear(&suite.failed, 0);
> +	strbuf_release(&progpath);
>   
>   	return ret;
>   }