In snapbox, impl From<PathBuf> for Data ?

[dbanty]

I'm working on upgrading to snapbox 0.5, and the new method of comparing an output to a file in the path seems to require more boilerplate. Instead of .stdout_matches_path(my_path), I now need to do .stdout_matches(Data::read_from(&my_path, None)).

What do you think about implementing both From<&Path> and From<PathBuf> for data to remove that extra boilerplate?

If there is another, cleaner method already, please let me know! 😁

[epage]

Wanted to double check first: is it a path that could use the file![] macro?

My main concern is that &str already is support for Data and people are used to &str being treated as a path and so I could see people mixing things up.

I wonder if there are other options for shortening this, like a function you can pull in.

[dbanty]

These are relative paths, where I have some base directory I'm working with and then .joining different bits throughout the test. I don't think that file! would work there, I believe i'd have to duplicate the shared base path with each usage.

I didn't know that &str is treated as a path for Data, I expected that to be a more complicated way of comparing to a literal string 😅. I might be able to shorten my code and use format!() instead of .join() when handling paths, it just feels wrong.

[epage]

These are relative paths, where I have some base directory I'm working with and then .joining different bits throughout the test. I don't think that file! would work there, I believe i'd have to duplicate the shared base path with each usage.

Could you share the code?

I didn't know that &str is treated as a path for Data, I expected that to be a more complicated way of comparing to a literal string 😅. I might be able to shorten my code and use format!() instead of .join() when handling paths, it just feels wrong.

&str is treated as content. The fact the APIs mix &str and &Path would make us supporting both &str and &Path confusing.

[dbanty]

Could you share the code?

Sure, there are a ton of tests set up like this.

There's a data directory for each test which contains the files to operate on in their initial state, as well as their expected state after running the command. It also has text files with snapshots of stdout/stderr.

I'm open to any advice you have for making these tests more maintainable 😁. I only just discovered subset_matches and I'm starting to replace a bunch of code with that.

[epage]

So it looks like the code is generally organized as:

<test>.rs
<test>/

<test>.rs
<test>/<case>/*

<test1>_<test2>.rs
<test1>/<test2>/<case>/*

In Cargo, we have it organized as

tests/testsuite/<cmd>/<case>/mod.rs
tests/testsuite/<cmd>/<case>/stdout.log
tests/testsuite/<cmd>/<case>/sterr.log
tests/testsuite/<cmd>/<case>/in
tests/testsuite/<cmd>/<case>/out

See https://github.com/rust-lang/cargo/tree/master/tests/testsuite/cargo_add/build_prefer_existing_version as an example

This is maybe a bit extreme where you have one test case per mod but it makes it easy to copy/paste the entire result for adding a new case.

The intention behind the API changes include

• Reducing digging into paths by leveraging where you are currently at.
• Putting users in control of working with structured data or not
• Simplifying the API and maintenance

None of this is to say "you are doing it wrong" but to explore the space and see what options, on both sides, look like.

I'm open to any advice you have for making these tests more maintainable 😁. I only just discovered subset_matches and I'm starting to replace a bunch of code with that.

Something I've been considering is having something like Data for directories and allowing the fixture and the snapshot to be in-Rust strings that contains txtar. Having the directories on disk was designed for when you can easily create a fixture from a reproduction case or debug a fixture live. The snapshots however are sometimes subsets that aren't always usable on their own, so you don't get the fully value (except maybe directory diffing). Also, in cargo's case, the fixtures can't be used on their own, requiring a lot of extra setup so we aren't getting as much value as I had intended.

Unsure if anything in this direction could help with your testing or have ideas for where else to take it.

[dbanty]

Wow, thanks so much for the detailed advice 😍! I was already starting to tinker with a more standard setup for tests to avoid boilerplate, and that mod pattern to complete the test-as-directory structure is just what I needed! Even better that it's working for a project as large as Cargo.

Because my tests mostly follow a standard pattern, I also was able to make a relatively simple API for tests to leverage, which looks something like this:

TestCase::new(file!())
        .git(&[
            Commit("Initial commit"),
            Tag("v1.0.0"),
            Commit("feat: New feature in existing release"),
            Tag("v1.1.0"),
            Commit("feat!: Breaking feature in new RC"),
        ])
        .env("KNOPE_PRERELEASE_LABEL", "alpha")
        .run("prerelease");

And the Data::read_froms that I need are all neatly tucked away for most cases, so implementing From<PathBuf> or similar is definitely not important anymore.