Skip to content

Conversation

@Zalathar
Copy link
Member

As more and more snapshot tests have been added or converted, this submodule has grown large enough that it no longer makes sense as an inline submodule. Pulling it out into its own file should make navigation easier.

r? Kobzol (or bootstrap)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Sep 18, 2025
Having these tests in an inline module made it harder to navigate both the
snapshot tests and the surrounding module.
@Zalathar
Copy link
Member Author

Zalathar commented Sep 18, 2025

Forcibly regenerated all snapshots (via INSTA_UPDATE=force) to match the new indentation.

Now if I do git diff master head --color-moved --color-moved-ws=allow-indentation-change, everything shows up as a clean move.

@Kobzol
Copy link
Member

Kobzol commented Sep 18, 2025

Hmm, I agree with the general idea, although I would rather just make all or most tests be snapshot tests, and then no longer call them "snapshot tests", but just "tests" :) So I'd just move them out of the inline module and just keep them in the file, and convert all the remaining tests to also use snapshots where it makes sense (#146595 is a step in that direction).

@Zalathar
Copy link
Member Author

This PR is unlikely to be merged in its current form, so I'll just close it for now.

@Zalathar Zalathar closed this Sep 24, 2025
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 24, 2025
@Zalathar Zalathar deleted the snapshot branch September 24, 2025 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants