Migrate tests of cargo-init to snapbox

[weihanglo]

What does this PR try to resolve?

An attempt of migrating tests of cargo-init to snapbox.

How should we test and review this PR?

To review this PR, you may compare the old and the new version one by one. Files not listed in <test-name>.out are not asserted, so please help me make sure we don't miss any essential file to check.

Some redundant tests are covered by other tests or renamed.

Expand to see the list of merged/renamed tests.

• gitignore_appended_not_replaced -> merged into git_ignore_exists_no_conflicting_entries
• gitignore_added_newline_in_existing -> merged into git_ignore_exists_no_conflicting_entries
• gitignore_no_newline_in_new -> merged into simple_git
• terminating_newline_in_existing_git_ignore -> merged into simple_git_ignore_exists
• terminating_newline_in_new_git_ignore -> merged into simple_git
• terminating_newline_in_new_mercurial_ignore -> merged into simple_hg
• terminating_newline_in_existing_mercurial_ignore -> merged into simple_hg_ignore_exists
• mercurial_added_newline_in_existing -> merged into simple_hg_ignore_exists
• mercurial_no_newline_in_new -> merged into simple_hg
• cargo_lock_gitignored_if_lib1 -> merged into simple_git
• cargo_lock_gitignored_if_lib2 -> renamed to inferred_lib_with_git
• cargo_lock_not_gitignored_if_bin2 -> renamed to inferred_bin_with_git
• cargo_lock_not_gitignored_if_bin1 -> renamed to explicit_bin_with_git

Additional information

I won't say the process of the migration was pleasant, but overall it results to a higher coverage of output file changes. Here are steps I performed to migrate a test case:

1. Run the old test and observe its output layout. Recommend using a separate worktree to preserve the temporary test output files.
2. Read the test code to arrange its input fixture at <test-name>.in.
3. Copy the old output layout or hand-pick the output layout you need. You don't need to fill contents of those files. snapbox will do it for you.
4. Run SNAPSHOTS=overwrite cargo test --test testsuite <your-test-filter> to assert and generate snapshots.
5. Compare the old layout and the new layout to see if anything missing.

Something observations when dealing with the migration:

• snapbox hasn't yet support unordered assertion.
• snapbox cannot assert inexistence of a file (And probably never?).
• No performance hit so far (measured with hyperfine).

[rust-highfive]

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

[ehuss]

Overall, I'm feeling very wary of making changes like this. I'm finding that this adds over 500 lines of code, and nearly 200 files, and adds quite a bit a complexity for not a substantial amount of benefit. Can we take a step back and evaluate what the strategy here is? I don't think we should be bulk updating tests unless there is in balance a benefit. The init tests for example have almost no output, and I don't feel like they benefit much from placing the output in separate files. I would also like to see snapbox mature more before migrating more tests.

If the primary concern is being able to update the output of a test, would an alternative be to just have a tool to update the strings within the test file automatically?

[epage]

Overall, I'm feeling very wary of making changes like this. I'm finding that this adds over 500 lines of code, and nearly 200 files, and adds quite a bit a complexity for not a substantial amount of benefit. Can we take a step back and evaluate what the strategy here is? I don't think we should be bulk updating tests unless there is in balance a benefit.

I think I might have misunderstood where people stood previously. I had thought we had intended to switch our testing over to snapbox.

Also, what are you finding is complex with this?

For me, the benefits are

• Increased coverage
• More ui testing
• Easier to update

The init tests for example have almost no output, and I don't feel like they benefit much from placing the output in separate files

I would also like to see snapbox mature more before migrating more tests.

What maturing would you like to see? it looks like this was chosen because this isn't hitting an area of missing features in snapbox. While I had intended to wait until those missing features are handled (unordered, structured data), I think an exercise like this is useful to see how well it works and to collect feedback, to iterate.

If the primary concern is being able to update the output of a test, would an alternative be to just have a tool to update the strings within the test file automatically?

The trade offs I see

• In-code: easier for someone to include a test function to reproduce an issue (though it happens, I feel like reporters rarely do this)
• File: easy to create a test from a reproduction case
• File: easy to update
• Both: easy to inspect and debug in the temp directory

In-code snapshot updating can get complicated I know the xflags arg parser does it and is relatively simple but that is more limited to when you have a singleton. insta provides the more complicated cases today. it makes different trade offs for its external snapshots, using a single managed file in a custom format that has the downsides to an external snapshot but not the upsides of working with real files. I'd be willing to brainstorm solutions and evaluate trade offs for what we want moving forward.

[epage]

@bors r+

Once this is merged, plan to make improvements

• Snapshots don't sort well in contexts (like IDEs) that sort by extension #10626
• Hard to review snapshots because they are far from their intended code #10627

And document when to use filesystem snapshots vs in-code: #10628

[bors]

📌 Commit 20c0c58 has been approved by epage

[bors]

⌛ Testing commit 20c0c58 with merge ad05cec...

[bors]

☀️ Test successful - checks-actions
Approved by: epage
Pushing ad05cec to master...