Skip to content

test-runner: rework output dir construction #17167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

robn
Copy link
Member

@robn robn commented Mar 22, 2025

[Sponsors: Klara, Inc., Wasabi Technology, Inc.]

Motivation and Context

Test output dirs are inconsistent. Sometimes things go under functional/, sometimes not, and single test mode doesn't always do the same thing as the full test suite, even though the runfile group appears the same.

Description

The old code would compare all the test group names to work out some sort of common path, but it didn't appear to work consistently, sometimes placing output in a top-level dir, other times in one or more subdirs. (I confess, I do not quite understand what it's supposed to do).

This is a very simple rework that simply looks at all the test group paths, removes common leading components, and uses the remainder as the output directory. This should work because groups paths are unique, and means we get a output dir tree of roughly the same shape as the test groups in the runfiles and the test source dirs themselves.

How Has This Been Tested?

Playing with running different tests and comparing output. The main comparison is a full ZTS run and comparing the output dir structure with one from a few days ago: before after.

Simple example. The acl tag has some subgroups, alloc_class does not. Without this change:

/var/tmp/test_results/20250216T104228/acl/off
/var/tmp/test_results/20250216T104228/acl/posix:Linux
/var/tmp/test_results/20250216T104228/acl/posix-sa:Linux
/var/tmp/test_results/20250216T104228/functional/alloc_class

With:

/var/tmp/test_results/20250321T152726/output/acl/off
/var/tmp/test_results/20250321T152726/output/acl/posix:Linux
/var/tmp/test_results/20250321T152726/output/acl/posix-sa:Linux
/var/tmp/test_results/20250321T152726/output/alloc_class

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@robn robn force-pushed the zts-sensible-output-dirs branch from c807046 to a7cd900 Compare March 22, 2025 02:02
@robn robn force-pushed the zts-sensible-output-dirs branch from a7cd900 to fa708d1 Compare April 2, 2025 01:24
@robn robn force-pushed the zts-sensible-output-dirs branch from fa708d1 to 5b4e855 Compare April 9, 2025 10:44
@tonyhutter
Copy link
Contributor

I like that the new results are sorted, but I'm not a huge fan of the added output/ directory. It's just one more directory you need to traverse to get to your results.

@robn
Copy link
Member Author

robn commented May 3, 2025

The output dir is there because there are other files directly under the top dir (perf_data, log, results, etc) and I don't want any conflicts, now or in the future.

But, it's also not much difference in most cases, because currently most tests get a functional/ top-level dir, which we no longer produce.

I personally don't care a whole lot, just so long as a test always gets the same output dir regardless of how you run it. So if you insist, I'll change it, and if we ever get a clash in the future, we can deal with it then.

The old code would compare all the test group names to work out some
sort of common path, but it didn't appear to work consistently,
sometimes placing output in a top-level dir, other times in one or more
subdirs. (I confess, I do not quite understand what it's supposed to
do).

This is a very simple rework that simply looks at all the test group
paths, removes common leading components, and uses the remainder as the
output directory. This should work because groups paths are unique, and
means we get a output dir tree of roughly the same shape as the test
groups in the runfiles and the test source dirs themselves.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <[email protected]>
@robn robn force-pushed the zts-sensible-output-dirs branch from 5b4e855 to b2826db Compare May 3, 2025 04:53
@tonyhutter
Copy link
Contributor

@robn I gave this a test and turns out my comment is unfounded - the main test results still show up in:

/var/tmp/test_results/current/log

... just like before. 99.9% of the time I just look at that, so the the output/ directory isn't really an issue.

@tonyhutter tonyhutter added the Status: Accepted Ready to integrate (reviewed, tested) label May 8, 2025
@robn
Copy link
Member Author

robn commented May 9, 2025

@tonyhutter thank you! Phew, because I was trying to imagine your workflow and I couldn't quite see it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants