Repository navigation
Switch from insta to expect-test in bootstrap - #164856
Conversation
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
Should/did we ask upstream on whether that's something they'd like to change? |
|
mitsuhiko/insta#490 it's a known issue, but it seems like there hasn't been activity on it. |
|
@bors r+ rollup |
…uwer Rollup of 20 pull requests Successful merges: - #163730 (add tests over re-entrancy guarantees for 160776) - #164834 (miri subtree update) - #164849 (Use the span of the doctest code block fence for figuring out the line number) - #164883 (rustdoc: don't panic on a nested use group with no path prefix) - #164890 (Store cold blocks in a bitset instead of a Vec of bools) - #163458 (stabilize {Rc, Arc}::is_unique) - #163499 (vec: stabilise allocators for into_iter) - #163805 (std: check `SO_ERROR` after poll in `TcpStream::connect_timeout`) - #163816 (stabilize String::into_chars) - #163819 (Correct documentation of what `core::ops::Code` is.) - #163847 (add regression test for reborrow-invariant-type) - #164006 (Arduously fix typo) - #164009 (Update `Ready` future expect messages) - #164522 (Add IPv6 hop limit stubs to the motor OS net backend) - #164602 (Improve seek_read_buf_exact() test coverage to match seek_read_exact()) - #164772 (docs: Two nitpick changes to `HashMap`, `BTreeMap` Entry API docs) - #164794 (Expose FloatErrorKind) - #164802 (Migrate stable-specific run-make tests to UI tests) - #164899 (Only recover if we may in `parse_expr_tuple_parens`) - #164903 (rustc-dev-guide subtree update) Failed merges: - #164856 (Switch from `insta` to `expect-test` in bootstrap)
Switch from `insta` to `expect-test` in bootstrap While I like `insta` a bit more for snapshot tests, it has a disadvantage that it requires installing a separate binary (`cargo insta`) for blessing *inline* snapshot tests. That is not very intuitive for people who need to bless inline bootstrap snapshots, and then realize that `x test bootstrap --bless` does nothing. This PR switches from `insta` to `expect-test`, which allows updating inline snapshots via an environment variable while running `cargo test`, without requiring a separate binary. `expect-test` is a `rust-lang` (well, `rust-analyzer`) [repository](https://github.com/rust-analyzer/expect-test), so we should be good there, and it even removes two `dev` dependencies from bootstrap. Changing the inline snapshots was a bit non-trivial to do with a regex (at least in my IDE) because we have to switch the argument order, which is not that simple across multiple lines. So I decided to try https://github.com/ast-grep/ast-grep. I used an LLM to propose the initial rewrite rule, and then modified it slightly and ran `ast-grep` to update the code. The applied rule was this: ```yaml id: insta-to-expect-test language: rust rule: pattern: insta::assert_snapshot!($$$ARGS) transform: LHS: replace: source: $$$ARGS replace: '(?s)^(?<lhs>.*?),\s*@(?<rhs>.*?)\s*,?\s*$' by: '${lhs}' RHS: replace: source: $$$ARGS replace: '(?s)^(?<lhs>.*?),\s*@(?<rhs>.*?)\s*,?\s*$' by: '${rhs}' fix: expect_test::expect![[$RHS]].assert_eq(&$LHS) ``` You can test it in the `ast-grep` [playground](https://ast-grep.github.io/playground.html#eyJtb2RlIjoiQ29uZmlnIiwibGFuZyI6InJ1c3QiLCJxdWVyeSI6IiIsInJld3JpdGUiOiJzb3VyY2U6ICQkJEFSR1NcbnJlcGxhY2U6ICcoP3MpXig/PGxocz4uKj8pLFxccypAKD88cmhzPi4qPylcXHMqLD9cXHMqJCdcbmJ5OiAnJHtyaHN9JyIsInN0cmljdG5lc3MiOiJzbWFydCIsInNlbGVjdG9yIjoiIiwiY29uZmlnIjoiaWQ6IGluc3RhLXRvLWV4cGVjdC10ZXN0XG5sYW5ndWFnZTogcnVzdFxucnVsZTpcbiAgcGF0dGVybjogaW5zdGE6OmFzc2VydF9zbmFwc2hvdCEoJCQkQVJHUylcbnRyYW5zZm9ybTpcbiAgTEhTOlxuICAgIHJlcGxhY2U6XG4gICAgICBzb3VyY2U6ICQkJEFSR1NcbiAgICAgIHJlcGxhY2U6ICcoP3MpXig/PGxocz4uKj8pLFxccypAKD88cmhzPi4qPylcXHMqLD9cXHMqJCdcbiAgICAgIGJ5OiAnJHtsaHN9J1xuICBSSFM6XG4gICAgcmVwbGFjZTpcbiAgICAgIHNvdXJjZTogJCQkQVJHU1xuICAgICAgcmVwbGFjZTogJyg/cyleKD88bGhzPi4qPyksXFxzKkAoPzxyaHM+Lio/KVxccyosP1xccyokJ1xuICAgICAgYnk6ICcke3Joc30nXG5maXg6IGV4cGVjdF90ZXN0OjpleHBlY3QhWyRSSFNdLmFzc2VydF9lcSgmJExIUykiLCJzb3VyY2UiOiJpbnN0YTo6YXNzZXJ0X3NuYXBzaG90IShcbiAgICAgICAgICAgIGN0eC5jb25maWcoXCJidWlsZFwiKVxuICAgICAgICAgICAgICAgIC5yZW5kZXJfc3RlcHMoKSwgQHJcIlxuICAgICAgICBbYnVpbGRdIGxsdm0gPGhvc3Q+XG4gICAgICAgIFtidWlsZF0gcnVzdGMgMCA8aG9zdD4gLT4gcnVzdGMgMSA8aG9zdD5cbiAgICAgICAgW2J1aWxkXSBydXN0YyAxIDxob3N0PiAtPiBzdGQgMSA8aG9zdD5cbiAgICAgICAgW2J1aWxkXSBydXN0ZG9jIDEgPGhvc3Q+XG4gICAgICAgIFwiLFxuICAgICAgICApOyJ9). I used double brackets because it formatted better (IMO) with `x fmt`. Most of the diff comes from the this mechanical change. I split it into a separate commit for easier review. I tried to replicate the disk snapshots as closely as possible, by even including the source file path in them, though I think it is not very useful. There are some newline changes in a few tests, I didn't feel like it is worth fixing those. CC @Zalathar I suppose that we should do the same for `citool`, I can send that as a follow-up. Fixes: rust-lang#164837
This comment has been minimized.
This comment has been minimized.
For a migration, these both seem like reasonable choices. 👍 |
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Rebased. @bors r=Mark-Simulacrum |
Switch from `insta` to `expect-test` in bootstrap While I like `insta` a bit more for snapshot tests, it has a disadvantage that it requires installing a separate binary (`cargo insta`) for blessing *inline* snapshot tests. That is not very intuitive for people who need to bless inline bootstrap snapshots, and then realize that `x test bootstrap --bless` does nothing. This PR switches from `insta` to `expect-test`, which allows updating inline snapshots via an environment variable while running `cargo test`, without requiring a separate binary. `expect-test` is a `rust-lang` (well, `rust-analyzer`) [repository](https://github.com/rust-analyzer/expect-test), so we should be good there, and it even removes two `dev` dependencies from bootstrap. Changing the inline snapshots was a bit non-trivial to do with a regex (at least in my IDE) because we have to switch the argument order, which is not that simple across multiple lines. So I decided to try https://github.com/ast-grep/ast-grep. I used an LLM to propose the initial rewrite rule, and then modified it slightly and ran `ast-grep` to update the code. The applied rule was this: ```yaml id: insta-to-expect-test language: rust rule: pattern: insta::assert_snapshot!($$$ARGS) transform: LHS: replace: source: $$$ARGS replace: '(?s)^(?<lhs>.*?),\s*@(?<rhs>.*?)\s*,?\s*$' by: '${lhs}' RHS: replace: source: $$$ARGS replace: '(?s)^(?<lhs>.*?),\s*@(?<rhs>.*?)\s*,?\s*$' by: '${rhs}' fix: expect_test::expect![[$RHS]].assert_eq(&$LHS) ``` You can test it in the `ast-grep` [playground](https://ast-grep.github.io/playground.html#eyJtb2RlIjoiQ29uZmlnIiwibGFuZyI6InJ1c3QiLCJxdWVyeSI6IiIsInJld3JpdGUiOiJzb3VyY2U6ICQkJEFSR1NcbnJlcGxhY2U6ICcoP3MpXig/PGxocz4uKj8pLFxccypAKD88cmhzPi4qPylcXHMqLD9cXHMqJCdcbmJ5OiAnJHtyaHN9JyIsInN0cmljdG5lc3MiOiJzbWFydCIsInNlbGVjdG9yIjoiIiwiY29uZmlnIjoiaWQ6IGluc3RhLXRvLWV4cGVjdC10ZXN0XG5sYW5ndWFnZTogcnVzdFxucnVsZTpcbiAgcGF0dGVybjogaW5zdGE6OmFzc2VydF9zbmFwc2hvdCEoJCQkQVJHUylcbnRyYW5zZm9ybTpcbiAgTEhTOlxuICAgIHJlcGxhY2U6XG4gICAgICBzb3VyY2U6ICQkJEFSR1NcbiAgICAgIHJlcGxhY2U6ICcoP3MpXig/PGxocz4uKj8pLFxccypAKD88cmhzPi4qPylcXHMqLD9cXHMqJCdcbiAgICAgIGJ5OiAnJHtsaHN9J1xuICBSSFM6XG4gICAgcmVwbGFjZTpcbiAgICAgIHNvdXJjZTogJCQkQVJHU1xuICAgICAgcmVwbGFjZTogJyg/cyleKD88bGhzPi4qPyksXFxzKkAoPzxyaHM+Lio/KVxccyosP1xccyokJ1xuICAgICAgYnk6ICcke3Joc30nXG5maXg6IGV4cGVjdF90ZXN0OjpleHBlY3QhWyRSSFNdLmFzc2VydF9lcSgmJExIUykiLCJzb3VyY2UiOiJpbnN0YTo6YXNzZXJ0X3NuYXBzaG90IShcbiAgICAgICAgICAgIGN0eC5jb25maWcoXCJidWlsZFwiKVxuICAgICAgICAgICAgICAgIC5yZW5kZXJfc3RlcHMoKSwgQHJcIlxuICAgICAgICBbYnVpbGRdIGxsdm0gPGhvc3Q+XG4gICAgICAgIFtidWlsZF0gcnVzdGMgMCA8aG9zdD4gLT4gcnVzdGMgMSA8aG9zdD5cbiAgICAgICAgW2J1aWxkXSBydXN0YyAxIDxob3N0PiAtPiBzdGQgMSA8aG9zdD5cbiAgICAgICAgW2J1aWxkXSBydXN0ZG9jIDEgPGhvc3Q+XG4gICAgICAgIFwiLFxuICAgICAgICApOyJ9). I used double brackets because it formatted better (IMO) with `x fmt`. Most of the diff comes from the this mechanical change. I split it into a separate commit for easier review. I tried to replicate the disk snapshots as closely as possible, by even including the source file path in them, though I think it is not very useful. There are some newline changes in a few tests, I didn't feel like it is worth fixing those. CC @Zalathar I suppose that we should do the same for `citool`, I can send that as a follow-up. Fixes: rust-lang#164837
…uwer Rollup of 8 pull requests Successful merges: - #164955 (Use the updated buffer length in UEFI locate_handles) - #164958 (uefi: roll back env changes when `start_image` fails) - #164968 (thread_local::guard: update comments a bit) - #164303 (Remove the 'sub' method from 'At' and migrate to 'sub_raw') - #164674 (Use a NoSpan marker for region constraints without spans) - #164856 (Switch from `insta` to `expect-test` in bootstrap) - #164934 (Remove redundant vec and duplicate indexing in find_noop_landing_pads) - #164954 (Reduce `SpanData` use)
|
💔 I suspect this PR failed tests as part of a rollup After fixing the problem, consider running a try job for the failed job before re-approving. Link to failure: #164974 (comment) |
|
This pull request was unapproved. This PR was contained in a rollup (#164974), which was unapproved. |
@bors try jobs=test-x86_64-msvc-2 |
Switch from `insta` to `expect-test` in bootstrap try-job: test-x86_64-msvc-2
This comment has been minimized.
This comment has been minimized.
|
Ok, another try. @bors r=Mark-Simulacrum |
Switch from `insta` to `expect-test` in bootstrap While I like `insta` a bit more for snapshot tests, it has a disadvantage that it requires installing a separate binary (`cargo insta`) for blessing *inline* snapshot tests. That is not very intuitive for people who need to bless inline bootstrap snapshots, and then realize that `x test bootstrap --bless` does nothing. This PR switches from `insta` to `expect-test`, which allows updating inline snapshots via an environment variable while running `cargo test`, without requiring a separate binary. `expect-test` is a `rust-lang` (well, `rust-analyzer`) [repository](https://github.com/rust-analyzer/expect-test), so we should be good there, and it even removes two `dev` dependencies from bootstrap. Changing the inline snapshots was a bit non-trivial to do with a regex (at least in my IDE) because we have to switch the argument order, which is not that simple across multiple lines. So I decided to try https://github.com/ast-grep/ast-grep. I used an LLM to propose the initial rewrite rule, and then modified it slightly and ran `ast-grep` to update the code. The applied rule was this: ```yaml id: insta-to-expect-test language: rust rule: pattern: insta::assert_snapshot!($$$ARGS) transform: LHS: replace: source: $$$ARGS replace: '(?s)^(?<lhs>.*?),\s*@(?<rhs>.*?)\s*,?\s*$' by: '${lhs}' RHS: replace: source: $$$ARGS replace: '(?s)^(?<lhs>.*?),\s*@(?<rhs>.*?)\s*,?\s*$' by: '${rhs}' fix: expect_test::expect![[$RHS]].assert_eq(&$LHS) ``` You can test it in the `ast-grep` [playground](https://ast-grep.github.io/playground.html#eyJtb2RlIjoiQ29uZmlnIiwibGFuZyI6InJ1c3QiLCJxdWVyeSI6IiIsInJld3JpdGUiOiJzb3VyY2U6ICQkJEFSR1NcbnJlcGxhY2U6ICcoP3MpXig/PGxocz4uKj8pLFxccypAKD88cmhzPi4qPylcXHMqLD9cXHMqJCdcbmJ5OiAnJHtyaHN9JyIsInN0cmljdG5lc3MiOiJzbWFydCIsInNlbGVjdG9yIjoiIiwiY29uZmlnIjoiaWQ6IGluc3RhLXRvLWV4cGVjdC10ZXN0XG5sYW5ndWFnZTogcnVzdFxucnVsZTpcbiAgcGF0dGVybjogaW5zdGE6OmFzc2VydF9zbmFwc2hvdCEoJCQkQVJHUylcbnRyYW5zZm9ybTpcbiAgTEhTOlxuICAgIHJlcGxhY2U6XG4gICAgICBzb3VyY2U6ICQkJEFSR1NcbiAgICAgIHJlcGxhY2U6ICcoP3MpXig/PGxocz4uKj8pLFxccypAKD88cmhzPi4qPylcXHMqLD9cXHMqJCdcbiAgICAgIGJ5OiAnJHtsaHN9J1xuICBSSFM6XG4gICAgcmVwbGFjZTpcbiAgICAgIHNvdXJjZTogJCQkQVJHU1xuICAgICAgcmVwbGFjZTogJyg/cyleKD88bGhzPi4qPyksXFxzKkAoPzxyaHM+Lio/KVxccyosP1xccyokJ1xuICAgICAgYnk6ICcke3Joc30nXG5maXg6IGV4cGVjdF90ZXN0OjpleHBlY3QhWyRSSFNdLmFzc2VydF9lcSgmJExIUykiLCJzb3VyY2UiOiJpbnN0YTo6YXNzZXJ0X3NuYXBzaG90IShcbiAgICAgICAgICAgIGN0eC5jb25maWcoXCJidWlsZFwiKVxuICAgICAgICAgICAgICAgIC5yZW5kZXJfc3RlcHMoKSwgQHJcIlxuICAgICAgICBbYnVpbGRdIGxsdm0gPGhvc3Q+XG4gICAgICAgIFtidWlsZF0gcnVzdGMgMCA8aG9zdD4gLT4gcnVzdGMgMSA8aG9zdD5cbiAgICAgICAgW2J1aWxkXSBydXN0YyAxIDxob3N0PiAtPiBzdGQgMSA8aG9zdD5cbiAgICAgICAgW2J1aWxkXSBydXN0ZG9jIDEgPGhvc3Q+XG4gICAgICAgIFwiLFxuICAgICAgICApOyJ9). I used double brackets because it formatted better (IMO) with `x fmt`. Most of the diff comes from the this mechanical change. I split it into a separate commit for easier review. I tried to replicate the disk snapshots as closely as possible, by even including the source file path in them, though I think it is not very useful. There are some newline changes in a few tests, I didn't feel like it is worth fixing those. CC @Zalathar I suppose that we should do the same for `citool`, I can send that as a follow-up. Fixes: rust-lang#164837
Switch from `insta` to `expect-test` in bootstrap While I like `insta` a bit more for snapshot tests, it has a disadvantage that it requires installing a separate binary (`cargo insta`) for blessing *inline* snapshot tests. That is not very intuitive for people who need to bless inline bootstrap snapshots, and then realize that `x test bootstrap --bless` does nothing. This PR switches from `insta` to `expect-test`, which allows updating inline snapshots via an environment variable while running `cargo test`, without requiring a separate binary. `expect-test` is a `rust-lang` (well, `rust-analyzer`) [repository](https://github.com/rust-analyzer/expect-test), so we should be good there, and it even removes two `dev` dependencies from bootstrap. Changing the inline snapshots was a bit non-trivial to do with a regex (at least in my IDE) because we have to switch the argument order, which is not that simple across multiple lines. So I decided to try https://github.com/ast-grep/ast-grep. I used an LLM to propose the initial rewrite rule, and then modified it slightly and ran `ast-grep` to update the code. The applied rule was this: ```yaml id: insta-to-expect-test language: rust rule: pattern: insta::assert_snapshot!($$$ARGS) transform: LHS: replace: source: $$$ARGS replace: '(?s)^(?<lhs>.*?),\s*@(?<rhs>.*?)\s*,?\s*$' by: '${lhs}' RHS: replace: source: $$$ARGS replace: '(?s)^(?<lhs>.*?),\s*@(?<rhs>.*?)\s*,?\s*$' by: '${rhs}' fix: expect_test::expect![[$RHS]].assert_eq(&$LHS) ``` You can test it in the `ast-grep` [playground](https://ast-grep.github.io/playground.html#eyJtb2RlIjoiQ29uZmlnIiwibGFuZyI6InJ1c3QiLCJxdWVyeSI6IiIsInJld3JpdGUiOiJzb3VyY2U6ICQkJEFSR1NcbnJlcGxhY2U6ICcoP3MpXig/PGxocz4uKj8pLFxccypAKD88cmhzPi4qPylcXHMqLD9cXHMqJCdcbmJ5OiAnJHtyaHN9JyIsInN0cmljdG5lc3MiOiJzbWFydCIsInNlbGVjdG9yIjoiIiwiY29uZmlnIjoiaWQ6IGluc3RhLXRvLWV4cGVjdC10ZXN0XG5sYW5ndWFnZTogcnVzdFxucnVsZTpcbiAgcGF0dGVybjogaW5zdGE6OmFzc2VydF9zbmFwc2hvdCEoJCQkQVJHUylcbnRyYW5zZm9ybTpcbiAgTEhTOlxuICAgIHJlcGxhY2U6XG4gICAgICBzb3VyY2U6ICQkJEFSR1NcbiAgICAgIHJlcGxhY2U6ICcoP3MpXig/PGxocz4uKj8pLFxccypAKD88cmhzPi4qPylcXHMqLD9cXHMqJCdcbiAgICAgIGJ5OiAnJHtsaHN9J1xuICBSSFM6XG4gICAgcmVwbGFjZTpcbiAgICAgIHNvdXJjZTogJCQkQVJHU1xuICAgICAgcmVwbGFjZTogJyg/cyleKD88bGhzPi4qPyksXFxzKkAoPzxyaHM+Lio/KVxccyosP1xccyokJ1xuICAgICAgYnk6ICcke3Joc30nXG5maXg6IGV4cGVjdF90ZXN0OjpleHBlY3QhWyRSSFNdLmFzc2VydF9lcSgmJExIUykiLCJzb3VyY2UiOiJpbnN0YTo6YXNzZXJ0X3NuYXBzaG90IShcbiAgICAgICAgICAgIGN0eC5jb25maWcoXCJidWlsZFwiKVxuICAgICAgICAgICAgICAgIC5yZW5kZXJfc3RlcHMoKSwgQHJcIlxuICAgICAgICBbYnVpbGRdIGxsdm0gPGhvc3Q+XG4gICAgICAgIFtidWlsZF0gcnVzdGMgMCA8aG9zdD4gLT4gcnVzdGMgMSA8aG9zdD5cbiAgICAgICAgW2J1aWxkXSBydXN0YyAxIDxob3N0PiAtPiBzdGQgMSA8aG9zdD5cbiAgICAgICAgW2J1aWxkXSBydXN0ZG9jIDEgPGhvc3Q+XG4gICAgICAgIFwiLFxuICAgICAgICApOyJ9). I used double brackets because it formatted better (IMO) with `x fmt`. Most of the diff comes from the this mechanical change. I split it into a separate commit for easier review. I tried to replicate the disk snapshots as closely as possible, by even including the source file path in them, though I think it is not very useful. There are some newline changes in a few tests, I didn't feel like it is worth fixing those. CC @Zalathar I suppose that we should do the same for `citool`, I can send that as a follow-up. Fixes: rust-lang#164837
…uwer Rollup of 12 pull requests Successful merges: - #162808 (Enable `--generate-link-to-definition` rustdoc option on std docs) - #163219 (assumptions_on_binders: Test mixed ambiguous ORs) - #164985 (Only recover if we may for `recover_missing_dot`) - #161521 (Replace check_attr_crate_level with check_target for #[doc]) - #162530 (Move #[naked] attribute check to attribute parsing stage) - #164103 (trait solver: Resolve region vars before eager placeholder handling) - #164692 (move `ppcf128` lang item to `core::num::imp`) - #164702 (Point at `impl` through which associated function is provided) - #164776 (Account for trait object with implicit `'static` lifetime in field type being returned) - #164791 (Self param after const param) - #164856 (Switch from `insta` to `expect-test` in bootstrap) - #164888 (Add regression tests for #156380)
…uwer Rollup of 14 pull requests Successful merges: - #163219 (assumptions_on_binders: Test mixed ambiguous ORs) - #164985 (Only recover if we may for `recover_missing_dot`) - #161521 (Replace check_attr_crate_level with check_target for #[doc]) - #162530 (Move #[naked] attribute check to attribute parsing stage) - #164103 (trait solver: Resolve region vars before eager placeholder handling) - #164692 (move `ppcf128` lang item to `core::num::imp`) - #164702 (Point at `impl` through which associated function is provided) - #164741 (Use explicit target cpu in tests parsing generated asm) - #164767 (Add regression tests for transmutability issues) - #164776 (Account for trait object with implicit `'static` lifetime in field type being returned) - #164791 (Self param after const param) - #164856 (Switch from `insta` to `expect-test` in bootstrap) - #164888 (Add regression tests for #156380) - #164967 (Add codegen test for power of two implying nonzero with debuginfo)
…uwer Rollup of 20 pull requests Successful merges: - rust-lang/rust#163730 (add tests over re-entrancy guarantees for 160776) - rust-lang/rust#164834 (miri subtree update) - rust-lang/rust#164849 (Use the span of the doctest code block fence for figuring out the line number) - rust-lang/rust#164883 (rustdoc: don't panic on a nested use group with no path prefix) - rust-lang/rust#164890 (Store cold blocks in a bitset instead of a Vec of bools) - rust-lang/rust#163458 (stabilize {Rc, Arc}::is_unique) - rust-lang/rust#163499 (vec: stabilise allocators for into_iter) - rust-lang/rust#163805 (std: check `SO_ERROR` after poll in `TcpStream::connect_timeout`) - rust-lang/rust#163816 (stabilize String::into_chars) - rust-lang/rust#163819 (Correct documentation of what `core::ops::Code` is.) - rust-lang/rust#163847 (add regression test for reborrow-invariant-type) - rust-lang/rust#164006 (Arduously fix typo) - rust-lang/rust#164009 (Update `Ready` future expect messages) - rust-lang/rust#164522 (Add IPv6 hop limit stubs to the motor OS net backend) - rust-lang/rust#164602 (Improve seek_read_buf_exact() test coverage to match seek_read_exact()) - rust-lang/rust#164772 (docs: Two nitpick changes to `HashMap`, `BTreeMap` Entry API docs) - rust-lang/rust#164794 (Expose FloatErrorKind) - rust-lang/rust#164802 (Migrate stable-specific run-make tests to UI tests) - rust-lang/rust#164899 (Only recover if we may in `parse_expr_tuple_parens`) - rust-lang/rust#164903 (rustc-dev-guide subtree update) Failed merges: - rust-lang/rust#164856 (Switch from `insta` to `expect-test` in bootstrap)
Rollup merge of #164856 - Kobzol:expect-test, r=Mark-Simulacrum Switch from `insta` to `expect-test` in bootstrap While I like `insta` a bit more for snapshot tests, it has a disadvantage that it requires installing a separate binary (`cargo insta`) for blessing *inline* snapshot tests. That is not very intuitive for people who need to bless inline bootstrap snapshots, and then realize that `x test bootstrap --bless` does nothing. This PR switches from `insta` to `expect-test`, which allows updating inline snapshots via an environment variable while running `cargo test`, without requiring a separate binary. `expect-test` is a `rust-lang` (well, `rust-analyzer`) [repository](https://github.com/rust-analyzer/expect-test), so we should be good there, and it even removes two `dev` dependencies from bootstrap. Changing the inline snapshots was a bit non-trivial to do with a regex (at least in my IDE) because we have to switch the argument order, which is not that simple across multiple lines. So I decided to try https://github.com/ast-grep/ast-grep. I used an LLM to propose the initial rewrite rule, and then modified it slightly and ran `ast-grep` to update the code. The applied rule was this: ```yaml id: insta-to-expect-test language: rust rule: pattern: insta::assert_snapshot!($$$ARGS) transform: LHS: replace: source: $$$ARGS replace: '(?s)^(?<lhs>.*?),\s*@(?<rhs>.*?)\s*,?\s*$' by: '${lhs}' RHS: replace: source: $$$ARGS replace: '(?s)^(?<lhs>.*?),\s*@(?<rhs>.*?)\s*,?\s*$' by: '${rhs}' fix: expect_test::expect![[$RHS]].assert_eq(&$LHS) ``` You can test it in the `ast-grep` [playground](https://ast-grep.github.io/playground.html#eyJtb2RlIjoiQ29uZmlnIiwibGFuZyI6InJ1c3QiLCJxdWVyeSI6IiIsInJld3JpdGUiOiJzb3VyY2U6ICQkJEFSR1NcbnJlcGxhY2U6ICcoP3MpXig/PGxocz4uKj8pLFxccypAKD88cmhzPi4qPylcXHMqLD9cXHMqJCdcbmJ5OiAnJHtyaHN9JyIsInN0cmljdG5lc3MiOiJzbWFydCIsInNlbGVjdG9yIjoiIiwiY29uZmlnIjoiaWQ6IGluc3RhLXRvLWV4cGVjdC10ZXN0XG5sYW5ndWFnZTogcnVzdFxucnVsZTpcbiAgcGF0dGVybjogaW5zdGE6OmFzc2VydF9zbmFwc2hvdCEoJCQkQVJHUylcbnRyYW5zZm9ybTpcbiAgTEhTOlxuICAgIHJlcGxhY2U6XG4gICAgICBzb3VyY2U6ICQkJEFSR1NcbiAgICAgIHJlcGxhY2U6ICcoP3MpXig/PGxocz4uKj8pLFxccypAKD88cmhzPi4qPylcXHMqLD9cXHMqJCdcbiAgICAgIGJ5OiAnJHtsaHN9J1xuICBSSFM6XG4gICAgcmVwbGFjZTpcbiAgICAgIHNvdXJjZTogJCQkQVJHU1xuICAgICAgcmVwbGFjZTogJyg/cyleKD88bGhzPi4qPyksXFxzKkAoPzxyaHM+Lio/KVxccyosP1xccyokJ1xuICAgICAgYnk6ICcke3Joc30nXG5maXg6IGV4cGVjdF90ZXN0OjpleHBlY3QhWyRSSFNdLmFzc2VydF9lcSgmJExIUykiLCJzb3VyY2UiOiJpbnN0YTo6YXNzZXJ0X3NuYXBzaG90IShcbiAgICAgICAgICAgIGN0eC5jb25maWcoXCJidWlsZFwiKVxuICAgICAgICAgICAgICAgIC5yZW5kZXJfc3RlcHMoKSwgQHJcIlxuICAgICAgICBbYnVpbGRdIGxsdm0gPGhvc3Q+XG4gICAgICAgIFtidWlsZF0gcnVzdGMgMCA8aG9zdD4gLT4gcnVzdGMgMSA8aG9zdD5cbiAgICAgICAgW2J1aWxkXSBydXN0YyAxIDxob3N0PiAtPiBzdGQgMSA8aG9zdD5cbiAgICAgICAgW2J1aWxkXSBydXN0ZG9jIDEgPGhvc3Q+XG4gICAgICAgIFwiLFxuICAgICAgICApOyJ9). I used double brackets because it formatted better (IMO) with `x fmt`. Most of the diff comes from the this mechanical change. I split it into a separate commit for easier review. I tried to replicate the disk snapshots as closely as possible, by even including the source file path in them, though I think it is not very useful. There are some newline changes in a few tests, I didn't feel like it is worth fixing those. CC @Zalathar I suppose that we should do the same for `citool`, I can send that as a follow-up. Fixes: #164837
…uwer Rollup of 14 pull requests Successful merges: - rust-lang/rust#163219 (assumptions_on_binders: Test mixed ambiguous ORs) - rust-lang/rust#164985 (Only recover if we may for `recover_missing_dot`) - rust-lang/rust#161521 (Replace check_attr_crate_level with check_target for #[doc]) - rust-lang/rust#162530 (Move #[naked] attribute check to attribute parsing stage) - rust-lang/rust#164103 (trait solver: Resolve region vars before eager placeholder handling) - rust-lang/rust#164692 (move `ppcf128` lang item to `core::num::imp`) - rust-lang/rust#164702 (Point at `impl` through which associated function is provided) - rust-lang/rust#164741 (Use explicit target cpu in tests parsing generated asm) - rust-lang/rust#164767 (Add regression tests for transmutability issues) - rust-lang/rust#164776 (Account for trait object with implicit `'static` lifetime in field type being returned) - rust-lang/rust#164791 (Self param after const param) - rust-lang/rust#164856 (Switch from `insta` to `expect-test` in bootstrap) - rust-lang/rust#164888 (Add regression tests for #156380) - rust-lang/rust#164967 (Add codegen test for power of two implying nonzero with debuginfo)

View all comments
While I like
instaa bit more for snapshot tests, it has a disadvantage that it requires installing a separate binary (cargo insta) for blessing inline snapshot tests. That is not very intuitive for people who need to bless inline bootstrap snapshots, and then realize thatx test bootstrap --blessdoes nothing.This PR switches from
instatoexpect-test, which allows updating inline snapshots via an environment variable while runningcargo test, without requiring a separate binary.expect-testis arust-lang(well,rust-analyzer) repository, so we should be good there, and it even removes twodevdependencies from bootstrap.Changing the inline snapshots was a bit non-trivial to do with a regex (at least in my IDE) because we have to switch the argument order, which is not that simple across multiple lines. So I decided to try https://github.com/ast-grep/ast-grep. I used an LLM to propose the initial rewrite rule, and then modified it slightly and ran
ast-grepto update the code. The applied rule was this:You can test it in the
ast-grepplayground.I used double brackets because it formatted better (IMO) with
x fmt. Most of the diff comes from the this mechanical change. I split it into a separate commit for easier review.I tried to replicate the disk snapshots as closely as possible, by even including the source file path in them, though I think it is not very useful. There are some newline changes in a few tests, I didn't feel like it is worth fixing those.
CC @Zalathar
I suppose that we should do the same for
citool, I can send that as a follow-up.Fixes: #164837