Skip to content

test: adopt rstest for parameterized tests - #73

Closed
remdalm wants to merge 2 commits into
silvermine:masterfrom
remdalm:feat/rstest
Closed

remdalm wants to merge 2 commits into
silvermine:masterfrom
remdalm:feat/rstest

Conversation

@remdalm

@remdalm remdalm commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

No description provided.

test_regular_abort_for_db_only_matching_db_key and
test_regular_abort_for_db_does_not_match_colon_prefix_alias ended by calling
abort_for_db for the other database. abort_for_db returns Ok(()) when nothing matches,
so neither test checked that the other transaction was still tracked.

Both tests now keep each task's AbortHandle. They assert that the matching task
finished and the other did not, then that aborting the other database finishes it.
Making abort_for_db match every transaction, or match by prefix, now fails them.
A test that loops over inputs, or puts one assert per input in one function, stops at
the first failing input, and cargo test cannot run one input on its own. rstest
generates one test per #[case], so a run reports every failing input and each one can
be run by name.

The scope follows tauri-plugin-connectivity's adoption of rstest (c5d624d), with a
stricter rule for merging separate tests:

- Every test that checked several inputs in one function is now a table with one
  named case per input.
- Separate tests are merged only when each case is another example of the same
  guarantee. A comment saying why an input is interesting is fine. A case that needs
  different assertions, a branch, or a comment arguing a different guarantee stays
  its own test.
- Scenario tests stay plain: setup loops, ordered or stateful steps, races, polling,
  and tests that differ in which operation they run.

Every input keeps its value. The storage-class round trip now reads each value
through quote(), which pins the value and its storage class in one string and
replaces the separate typeof probe. Some rejections now assert the exact error variant
or code instead of is_err(). test_reserved_schema_names_rejected loses its closing
check that nothing stayed attached: every rejection happens in validation, before
any ATTACH, so that check could not fail.

The workspace goes from 358 test functions to 414 generated tests, 171 of them cases
in 42 tables. CHANGELOG.md names the readable-notification test by its new name.

rstest 0.28.0 is a dev-dependency of all four crates with default features off. Its
crate-name feature changes feature resolution for toml crates that tauri's build
shares, so a warm workspace build would rebuild tauri. Cargo.lock adds rstest,
rstest_macros and relative-path. It also moves some Windows-only edges from
windows-sys 0.60 to 0.61, which adding any new package does here. Nothing else in the
normal dependency graph changes.
@remdalm
remdalm requested a review from a team October 20, 2026 11:49
@remdalm
remdalm marked this pull request as draft October 20, 2026 11:49
@remdalm
remdalm marked this pull request as ready for review October 20, 2026 12:07
@@ -0,0 +1,139 @@
/**

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 P2 — Remove the rstest lint script and the rust:lint:rstest step from this PR

The PR adds scripts/lint-rstest-attributes.cjs to rust:lint, but the tauri-plugin-connectivity adoption of rstest that this PR follows added no such script.

Details

The commit body says the scope follows tauri-plugin-connectivity's adoption of rstest (c5d624d). That commit changes only Rust sources, Cargo.toml and Cargo.lock. The connectivity rust:lint script is still cargo lint-clippy && cargo lint-fmt. The commit body of this PR does not mention the lint script.

The script has no tests and cannot be imported, because it calls main() on load and exports nothing. A later regex change can turn the check off without a signal. Fixtures also show three gaps: a #[cfg_attr(..., tokio::test)] above the cases is not detected, a multi-line #[case] string line that starts with fn ends the block early, and a #[ line inside a raw string or block comment gives a false positive.

Removing it removes the guard against dropped cases. rstest 0.27.0 and 0.28.0 both generate only case_1 when #[test] or #[tokio::test] is above the cases. Today all 42 #[rstest] functions in this PR use a safe order, and the 31 in tauri-plugin-connectivity do too.

Delete scripts/lint-rstest-attributes.cjs. In package.json, set rust:lint back to cargo lint-clippy && cargo lint-fmt and remove rust:lint:rstest. If the team wants this guard later, add it to shared tooling so that both plugins get it.

Comment added by AI model Claude

Comment thread Cargo.toml
# Parameterized tests. Default features stay off: `crate-name` changes feature
# resolution for toml crates that tauri's build shares, so a warm workspace build
# would rebuild tauri, and nothing here uses `async-timeout`.
rstest = { version = "0.28.0", default-features = false }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 P2 — Pin rstest to 0.27.0 to match tauri-plugin-connectivity

This PR adds rstest 0.28.0, but the only other Silvermine project that uses rstest, tauri-plugin-connectivity, uses 0.27.0.

Details

tauri-plugin-connectivity declares rstest = { version = "0.27.0", default-features = false } in crates/connectivity/Cargo.toml. The rust-coding-standards rule on new dependencies says to use what other Silvermine projects use, or to consult the team first. rstest 0.28.0 was published on 2026-10-18.

I verified the change locally: I set all four manifests to "0.27.0" and ran cargo update -p rstest --precise 0.27.0. Only rstest and rstest_macros change in Cargo.lock. cargo test --workspace --lib --test '*' gives 413 passed and 1 ignored, which is the same result as on 0.28.0. rstest 0.27.0 has the same default features (async-timeout, crate-name), so the default-features = false comment stays correct.

Change 0.28.0 to 0.27.0 here and in the three crate manifests, then run:

cargo update -p rstest --precise 0.27.0

Also update the sentence "rstest 0.28.0 is a dev-dependency of all four crates" in the commit body.

Comment added by AI model Claude

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Pending PR in tauri-plugin-connectivity to pin rstest to 0.28.0: silvermine/tauri-plugin-connectivity#38

#[case::where_in_line_comment("SELECT * FROM posts -- WHERE id > 5")]
// No real WHERE — the word WHERE is entirely inside a string with escaped quotes
#[case::where_only_in_escaped_quote_string("SELECT 'it''s WHERE we go' FROM t")]
fn has_top_level_where_ignores(#[case] query: &str) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 P3 — Rename has_top_level_where_ignores so the name states its purpose

The name has no object and does not describe the no_where_clause case, which coding-standards' naming rule requires; use has_top_level_where_returns_false.

Comment added by AI model Claude

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants