Re: [Valgrind-developers] [LTP] [PATCH v2] lib: tst_test: Add reproducible output.

[Petr V. <pv...@su...>]

Hi Cyril,

> This commit adds an environment variable LTP_REPRODUCIBLE_OUTPUT that
> when set skips printing parts of the test messages that may contain data
> that differ on subsequent runs (e.g. pids).

> With this you can run a test twice under a different conditions and
> check if the test codeflow was identical by simply doing diff of the
> outputs from the two runs.

LGTM, thanks!
Reviewed-by: Petr Vorel <pv...@su...>

Could you please add before the commit the variable to
print_help() and to the docs doc/users/setup_tests.rst?

I suggest the diff below, which also sort the entries (ideally it'd be
separated, but it's before the release, feel free to do it).

> Signed-off-by: Cyril Hrubis <ch...@su...>
> Suggested-by: Martin Doucha <md...@su...>
> CC: val...@li...
> ---
>  lib/tst_test.c | 7 +++++++
>  1 file changed, 7 insertions(+)

> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 2bb4519dd..f14627544 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -64,6 +64,7 @@ static int mntpoint_mounted;
>  static int ovl_mounted;
>  static struct timespec tst_start_time; /* valid only for test pid */
>  static int tdebug;
> +static int reproducible_output;

>  struct results {
>  	int passed;
> @@ -312,6 +313,9 @@ static void print_result(const char *file, const int lineno, int ttype,
>  	str += ret;
>  	size -= ret;

> +	if (reproducible_output)
> +		goto print;
> +
>  	ssize = size - 2;
>  	ret = vsnprintf(str, size, fmt, va);
>  	str += MIN(ret, ssize);
> @@ -329,6 +333,7 @@ static void print_result(const char *file, const int lineno, int ttype,
>  				"Next message is too long and truncated:");
>  	}

> +print:
>  	snprintf(str, size, "\n");

>  	/* we might be called from signal handler, so use write() */
> @@ -1312,6 +1317,8 @@ static void do_setup(int argc, char *argv[])
>  	if (tst_test->supported_archs && !tst_is_on_arch(tst_test->supported_archs))
>  		tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);

> +	reproducible_output = !!getenv("LTP_REPRODUCIBLE_OUTPUT");

LTP_COLORIZE_OUTPUT allows more complicated setup (y/1 always, n/0: never),
but any value here enables the feature (e.g. LTP_COLORIZE_OUTPUT=0).
This would be nice to unify, but I'm ok to ignore it as it's before release.

> +
>  	assert_test_fn();

>  	TCID = tid = get_tid(argv);

Adding docs (+ sort variables while at it).

Kind regards,
Petr

+++ doc/users/setup_tests.rst
@@ -49,11 +49,12 @@ users.
    * - LTP_DEV_FS_TYPE
      - Filesystem used for testing (default: ``ext2``).
 
-   * - LTP_TIMEOUT_MUL
-     - Multiplies timeout, must be number >= 0.1 (> 1 is useful for slow
-       machines to avoid unexpected timeout). It's mainly for shell API, which
-       does not have LTP_RUNTIME_MUL. In C API it scales the default 30 sec
-       safety margin, probably LTP_RUNTIME_MUL should be used instead.
+   * - LTP_IMA_LOAD_POLICY
+     - Load IMA example policy, see :master:`testcases/kernel/security/integrity/ima/README.md`.
+
+   * - LTP_REPRODUCIBLE_OUTPUT
+     - Dischard the actual content of the messages printed by the test
+       (suitable for a reproducible output).
 
    * - LTP_RUNTIME_MUL
      - Multiplies maximal test iteration runtime. Tests that run for more than a
@@ -61,8 +62,11 @@ users.
        both up and down with this multiplier. This is not yet implemented in the
        shell API.
 
-   * - LTP_IMA_LOAD_POLICY
-     - Load IMA example policy, see :master:`testcases/kernel/security/integrity/ima/README.md`.
+   * - LTP_TIMEOUT_MUL
+     - Multiplies timeout, must be number >= 0.1 (> 1 is useful for slow
+       machines to avoid unexpected timeout). It's mainly for shell API, which
+       does not have LTP_RUNTIME_MUL. In C API it scales the default 30 sec
+       safety margin, probably LTP_RUNTIME_MUL should be used instead.
 
    * - LTP_VIRT_OVERRIDE
      - Overrides virtual machine detection in the test library. Setting it to
diff --git lib/tst_test.c lib/tst_test.c
index f14627544f..a014ddc278 100644
--- lib/tst_test.c
+++ lib/tst_test.c
@@ -613,9 +613,10 @@ static void print_help(void)
 	fprintf(stderr, "LTP_COLORIZE_OUTPUT  Force colorized output behaviour (y/1 always, n/0: never)\n");
 	fprintf(stderr, "LTP_DEV              Path to the block device to be used (for .needs_device)\n");
 	fprintf(stderr, "LTP_DEV_FS_TYPE      Filesystem used for testing (default: %s)\n", DEFAULT_FS_TYPE);
+	fprintf(stderr, "LTP_REPRODUCIBLE_OUTPUT      Dischard the actual content of the messages printed by the test\n");
+	fprintf(stderr, "LTP_RUNTIME_MUL      Runtime multiplier (must be a number >=1)\n");
 	fprintf(stderr, "LTP_SINGLE_FS_TYPE   Testing only - specifies filesystem instead all supported (for .all_filesystems)\n");
 	fprintf(stderr, "LTP_TIMEOUT_MUL      Timeout multiplier (must be a number >=1)\n");
-	fprintf(stderr, "LTP_RUNTIME_MUL      Runtime multiplier (must be a number >=1)\n");
 	fprintf(stderr, "LTP_VIRT_OVERRIDE    Overrides virtual machine detection (values: \"\"|kvm|microsoft|xen|zvm)\n");
 	fprintf(stderr, "TMPDIR               Base directory for template directory (for .needs_tmpdir, default: %s)\n", TEMPDIR);
 	fprintf(stderr, "\n");