Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[test-infra][1/n] Replace Move's .exp tests with cargo-insta #21003

Merged
merged 10 commits into from
Jan 29, 2025
Merged

[test-infra][1/n] Replace Move's .exp tests with cargo-insta #21003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c8724b9
shared insta util
tnowacki Jan 24, 2025
6081b04
transition to macro
tnowacki Jan 29, 2025
07c4bc4
regen snapshots
tnowacki Jan 29, 2025
bb3bc0a
move files
tnowacki Jan 29, 2025
9a59f49
comments
tnowacki Jan 29, 2025
a76b82a
split into separate tests
tnowacki Jan 29, 2025
d359026
Add suffix option
tnowacki Jan 29, 2025
865db4e
more docs
tnowacki Jan 29, 2025
48c6713
more comments
tnowacki Jan 29, 2025
bebe54b
clippy
tnowacki Jan 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 21 additions & 2 deletions external-crates/move/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions external-crates/move/crates/move-command-line-common/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ serde.workspace = true
dirs-next.workspace = true
vfs.workspace = true
bcs.workspace = true
insta = { workspace = true, features = ["serde"] }

move-core-types.workspace = true
move-binary-format.workspace = true
Expand Down
129 changes: 129 additions & 0 deletions external-crates/move/crates/move-command-line-common/src/testing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,132 @@ pub fn format_diff(expected: impl AsRef<str>, actual: impl AsRef<str>) -> String
}
ret
}

/// See `insta_assert!` for documentation.
pub struct InstaOptions<Info: serde::Serialize, Suffix: Into<String>> {
pub info: Option<Info>,
pub suffix: Option<Suffix>,
}

impl<Info: serde::Serialize, Suffix: Into<String>> InstaOptions<Info, Suffix> {
/// See `insta_assert!` for documentation.
pub fn new() -> Self {
Self {
info: None,
suffix: None,
}
}
}

impl InstaOptions<(), String> {
/// See `insta_assert!` for documentation.
pub fn none() -> Self {
Self {
info: None,
suffix: None,
}
}
}

#[macro_export]
/// A wrapper around `insta::assert_snapshort` to promote uniformity in the Move codebase, intended
/// to be used with datatest-stable and as a replacement for the hand-rolled baseline tests.
/// The snapshot file will be saved in the same directory as the input file with the name specified.
/// In essence, it will be saved at the path `{input_path}/{name}.snap` (and
/// `{input_path}/{name}@{suffix}.snap` if `suffix` is specified).
///
/// For ease of use and reviewing, `insta_assert` should be used at most once per test. When it
/// fails, it will stop the test. So if there are multiple snapshots in a given test, it would
/// require multiple test runs to review all the failures.
Comment on lines +100 to +102
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this is necessarily true -- see "Disabling Assertion Failure" here: https://insta.rs/docs/advanced/#disabling-assertion-failure. Might be worth mentioning this/pointing to it in case folks do want to generate multiple snapshots per test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll mention it, but I think given the fact that you can only skip it via an env (and not something in Settings) it is worth advising against. Just a lot of friction and what not

/// If you do need multiple snapshots in a test, you may want to disable assertions for your test
/// run by setting `INSTA_FORCE_PASS=1` see
/// https://insta.rs/docs/advanced/#disabling-assertion-failure for more information.
///
/// # Arguments
/// The macro has three required arguments:
///
/// - `name`: The name of the test. This will be used to name the snapshot file. For datatest this
/// should likely be the file name.
/// - `input_path`: The path to the input file. This is used to determine the snapshot path.
/// - `contents`: The contents to snapshot.
///
///
/// The macro also accepts an optional arguments to that are used with `InstaOptions` to customize
/// the snapshot. If needed the `InstaOptions` struct can be used directly by specifying the
/// `options` argument. Options include:
///
/// - `info`: Additional information to include in the header of the snapshot file. This can be
/// useful for debugging tests. The value can be any type that implements
/// `serde::Serialize`.
/// - `suffix`: A suffix to append to the snapshot file name. This changes the snapshot path to
/// `{input_path}/{name}@{suffix}.snap`.
///
/// # Updating snapshots
///
/// After running the test, the `.snap` files can be updated in two ways:
///
/// 1. By using `cargo insta review`, which will open an interactive UI to review the changes.
/// 2. Running the tests with the environment variable `INSTA_UPDATE=alawys`
///
/// See https://docs.rs/insta/latest/insta/#updating-snapshots for more information.
macro_rules! insta_assert {
{
name: $name:expr,
input_path: $input:expr,
contents: $contents:expr,
options: $options:expr
$(,)?
} => {{
let name: String = $name.into();
let i: &std::path::Path = $input.as_ref();
let i = i.canonicalize().unwrap();
let c = $contents;
let $crate::testing::InstaOptions { info, suffix } = $options;
let mut settings = insta::Settings::clone_current();
settings.set_input_file(&i);
settings.set_snapshot_path(i.parent().unwrap());
if let Some(info) = info {
settings.set_info(info);
}
if let Some(suffix) = suffix {
settings.set_snapshot_suffix(suffix);
}
settings.set_prepend_module_to_snapshot(false);
settings.set_omit_expression(true);
settings.bind(|| {
insta::assert_snapshot!(name, c);
});
}};
{
name: $name:expr,
input_path: $input:expr,
contents: $contents:expr
$(,)?
} => {{
insta_assert! {
name: $name,
input_path: $input,
output_path: $output,
contents: $contents,
options: $crate::testing::InstaOptions::none(),
}
}};
{
name: $name:expr,
input_path: $input:expr,
contents: $contents:expr,
$($k:ident: $v:expr),+$(,)?
} => {{
let mut opts = $crate::testing::InstaOptions::new();
$(
opts.$k = Some($v);
)+
insta_assert! {
name: $name,
input_path: $input,
contents: $contents,
options: opts
}
}};
}
pub use insta_assert;
10 changes: 9 additions & 1 deletion ...otation__collapsed_sections__a__m.md.snap → ...nnotation/a__m.md@collapsed_sections.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
---
source: crates/move-docgen-tests/tests/testsuite.rs
expression: contents
info:
section_level_start: 1
exclude_private_fun: false
exclude_impl: false
toc_depth: 3
no_collapsed_sections: true
include_dep_diagrams: false
include_call_diagrams: false
input_file: crates/move-docgen-tests/tests/move/annotation/Move.toml
---
<a name="a_m"></a>

Expand Down
10 changes: 9 additions & 1 deletion ...__move__annotation__default__a__m.md.snap → ...ests/move/annotation/a__m.md@default.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
---
source: crates/move-docgen-tests/tests/testsuite.rs
expression: contents
info:
section_level_start: 1
exclude_private_fun: false
exclude_impl: false
toc_depth: 3
no_collapsed_sections: false
include_dep_diagrams: false
include_call_diagrams: false
input_file: crates/move-docgen-tests/tests/move/annotation/Move.toml
---
<a name="a_m"></a>

Expand Down
10 changes: 9 additions & 1 deletion ...e_block__collapsed_sections__a__m.md.snap → ...ode_block/a__m.md@collapsed_sections.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
---
source: crates/move-docgen-tests/tests/testsuite.rs
expression: contents
info:
section_level_start: 1
exclude_private_fun: false
exclude_impl: false
toc_depth: 3
no_collapsed_sections: true
include_dep_diagrams: false
include_call_diagrams: false
input_file: crates/move-docgen-tests/tests/move/code_block/Move.toml
---
<a name="a_m"></a>

Expand Down
10 changes: 9 additions & 1 deletion ...__move__code_block__default__a__m.md.snap → ...ests/move/code_block/a__m.md@default.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
---
source: crates/move-docgen-tests/tests/testsuite.rs
expression: contents
info:
section_level_start: 1
exclude_private_fun: false
exclude_impl: false
toc_depth: 3
no_collapsed_sections: false
include_dep_diagrams: false
include_call_diagrams: false
input_file: crates/move-docgen-tests/tests/move/code_block/Move.toml
---
<a name="a_m"></a>

Expand Down
10 changes: 9 additions & 1 deletion ...move__const_string__default__a__m.md.snap → ...st_string/a__m.md@collapsed_sections.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
---
source: crates/move-docgen-tests/tests/testsuite.rs
expression: contents
info:
section_level_start: 1
exclude_private_fun: false
exclude_impl: false
toc_depth: 3
no_collapsed_sections: true
include_dep_diagrams: false
include_call_diagrams: false
input_file: crates/move-docgen-tests/tests/move/const_string/Move.toml
---
<a name="a_m"></a>

Expand Down
10 changes: 9 additions & 1 deletion ..._string__collapsed_sections__a__m.md.snap → ...ts/move/const_string/a__m.md@default.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
---
source: crates/move-docgen-tests/tests/testsuite.rs
expression: contents
info:
section_level_start: 1
exclude_private_fun: false
exclude_impl: false
toc_depth: 3
no_collapsed_sections: false
include_dep_diagrams: false
include_call_diagrams: false
input_file: crates/move-docgen-tests/tests/move/const_string/Move.toml
---
<a name="a_m"></a>

Expand Down
10 changes: 9 additions & 1 deletion ...ilities__collapsed_sections__a__m.md.snap → ...ibilities/a__m.md@collapsed_sections.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
---
source: crates/move-docgen-tests/tests/testsuite.rs
expression: contents
info:
section_level_start: 1
exclude_private_fun: false
exclude_impl: false
toc_depth: 3
no_collapsed_sections: true
include_dep_diagrams: false
include_call_diagrams: false
input_file: crates/move-docgen-tests/tests/move/different_visibilities/Move.toml
---
<a name="a_TestViz"></a>

Expand Down
10 changes: 9 additions & 1 deletion ...erent_visibilities__default__a__m.md.snap → ...fferent_visibilities/a__m.md@default.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
---
source: crates/move-docgen-tests/tests/testsuite.rs
expression: contents
info:
section_level_start: 1
exclude_private_fun: false
exclude_impl: false
toc_depth: 3
no_collapsed_sections: false
include_dep_diagrams: false
include_call_diagrams: false
input_file: crates/move-docgen-tests/tests/move/different_visibilities/Move.toml
---
<a name="a_TestViz"></a>

Expand Down
10 changes: 9 additions & 1 deletion ...__enums__collapsed_sections__a__m.md.snap → ...ove/enums/a__m.md@collapsed_sections.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
---
source: crates/move-docgen-tests/tests/testsuite.rs
expression: contents
info:
section_level_start: 1
exclude_private_fun: false
exclude_impl: false
toc_depth: 3
no_collapsed_sections: true
include_dep_diagrams: false
include_call_diagrams: false
input_file: crates/move-docgen-tests/tests/move/enums/Move.toml
---
<a name="a_m"></a>

Expand Down
Loading
Loading