Test store dir relative to target dir changes

[eduardorittner]

Added both tests for parsing configuration and integration tests, let me know what you think! Currently, integration tests only check that the store dir was created in the right place, but not that any output was written there since I don't exactly know what's the right output to expect, but if you feel this is important I can take some more time and add more thorough checks.

[sunshowers]

Thanks for doing this!

I need to think more deeply about the consequences of this approach (not your tests which are great, but the general approach of using repo config for this.) I'm wondering if this is more of a per-user setting than a per-repo setting, or maybe if we should support both. Need to think more about this.

[eduardorittner]

Sure, no problem! Take your time

[sunshowers]

I've been thinking about this and I think I'd like to move towards deprecating the store directory feature instead. The only thing it is currently used for is the JUnit file, and what I want to do instead is to extend the recording feature to generate the JUnit file instead. Then, JUnit would be a two-step process:

1. Generate a recording.
2. Export JUnit from the recording.

Step 1 for that is to avoid creating the store directory if JUnit isn't enabled. I'll try getting that done soon.

[eduardorittner]

Hi! I haven't been following along with merged PRs and so on, have you already done this? If not I could take a stab at it (if that's of with you, of course).

[sunshowers]

I've deprecated the store directory in the sense that it's no longer created if JUnit support isn't enabled. Actually hooking up JUnit exports to run recordings is going to require a bit of care, particularly around how JUnit configuration in repo config should be read. For example, we may want to store the repo config in the run archive so that nextest can figure out which tests to include from that.

You're welcome to give that a go if you like!

[eduardorittner]

Hi! It's been a while, but it looks like this is still open, so I made a working POC which adds a new cargo command cargo nextest store export-junit [RUN_ID | latest | recording.zip] [-o PATH] [--report-name NAME] which exports a JUnit report from a recording. It works without junit.path and from portable recordings.

The way it was done is that JUnit policy settings are resolved independently from junit.path, and recordings with JUnit disabled carry the per-test values, instead of defaulting to all-false. The store format was updated to capture setup-script JUnit flags and resolved report-name. Old recordings (that is, recordings with an older version) omit setup-script output, and resolve report-name to "nextest-run". The JUnit aggregator was changed to JunitReportBuilder which doesn't need EvaluatableProfile, so it's usable without any repo config. Live events go through MetadataJunit which passes them through to the JunitReportBuilder and stores the output; recorded events are converted back to a live TestEvent and fed into JunitReportBuilder. This way, exported and live reports are (supposed to be) identical byte for byte.

If you agree with the overall changes, I can refine them a bit and then split them up into smaller PRs for more detailed review! I'm just showing the complete POC so you can get a sense of how things would roughly fit together.

If you like the overall design, I do have some questions:

1. You mentioned storing the repo config in the run archive as a suggestion. I opted to use the resolved values recorded in the test events because it seemed conceptually simpler: this way exports don't have to parse configs, evaluate per-test queries. Do you think this is acceptable? Two drawbacks of this approach are: the profile at export time is identical to the one at run time, meaning we can't "reinterpret" the tests with a different profile when exporting them.
2. Crashed runs still get a report, albeit with a warning. I'm not sure whether to make this a hard error instead.