Kill mutation-testing survivors (#55, #56, #57, #58, #59) - #63
Open
leynos wants to merge 8 commits into
Open
Conversation
added 4 commits
July 13, 2026 20:54
Address the survivors from the full mutation-testing dispatch run (dataset SHA 1004e60) harvested into issues #55-#59: - Config store (#55): exercise the real on-disk ConfigStore::current_volume_id path with a round-trip test and a missing-config test; previously only a test double was observed. - Init (#56): add unit tests for InitConfig::validate and a four-case table test for format_failure_message, and de-tautologize the BYTES_PER_GB conversion test by asserting the literal byte count. - Scaleway classification (#57): add unit tests for is_instance_type_error and validate_cache_volume_id, and delete the dead third clause of is_instance_type_error, which was subsumed by the first clause and could never affect the result. - Scaleway lifecycle (#58): introduce a fetch seam by extracting the polling loop bodies into generic poll_for_public_ip and poll_until_gone helpers parameterized over an instance-fetch closure. The wait tests now drive the production loops with scripted snapshots instead of a FakeBackend duplicating the algorithm. Add a happy-path readiness test, an already-gone teardown test, and a power_on_if_needed case where "poweron" is not among the allowed actions. - Sync (#59): assert the StrictHostKeyChecking and UserKnownHostsFile options are emitted and omitted as the configuration toggles. Add .cargo/mutants.toml excluding the cfg-gated test-backdoor scaffolding in src/main.rs (mirroring the existing src/test_support.rs exclude-glob) and the equivalent InstanceRequest::builder Default::default() mutant, with justifications inline.
The "replace <fn>" patterns missed operator mutants inside the test-backdoor functions (e.g. "replace == with != in enable_fake_modes"). Bare function-name substrings cover both the whole-function and in-function mutant name forms.
Scoped cargo-mutants confirmation runs surfaced three further missed mutants beyond the harvested worklist: - config_store path_exists: the NotFound match guard replaced with true silently treated any directory-open failure as "config does not exist". A new test probes a path whose parent is a regular file and asserts the NotADirectory failure surfaces as an Io error. - init FormatFailure::fmt replaced with Ok(Default::default()): assert the Display output carries the failure message. - init append_teardown_note replaced with a fixed string: assert the message passes through unchanged without a teardown error and that a teardown error is appended.
Clippy's too-many-arguments ceiling (four) rejected the five-argument poll_for_public_ip helper. Carry the SSH port and timing values in a small copyable settings struct instead.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary
Validation
WalkthroughThe PR extracts reusable Scaleway lifecycle polling helpers and adds regression coverage for lifecycle, configuration, initialisation, Scaleway classification, synchronisation, and mutation-testing behaviour. ChangesLifecycle and regression coverage
Possibly related issues
Possibly related PRs
Suggested labels: Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 2 warnings, 2 inconclusive)
✅ Passed checks (15 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideAdds targeted unit tests and small refactors to make previously surviving mutation-testing changes observable, deletes one subsumed predicate clause, and configures cargo-mutants exclusions for known scaffolding and an equivalent mutant, thereby closing real test gaps without altering production behaviour. Sequence diagram for ScalewayBackend wait_for_public_ip polling via generic helpersequenceDiagram
actor Caller
participant ScalewayBackend
participant poll_for_public_ip
participant fetch_closure
participant ScalewayAPI
Caller->>ScalewayBackend: wait_for_public_ip(handle)
ScalewayBackend->>poll_for_public_ip: poll_for_public_ip(handle, PollSettings, || fetch_instance(handle))
loop until deadline
poll_for_public_ip->>fetch_closure: fetch()
fetch_closure->>ScalewayBackend: fetch_instance(handle)
ScalewayBackend->>ScalewayAPI: list_instances(handle)
ScalewayAPI-->>ScalewayBackend: Result<Option<InstanceSnapshot>, ScalewayBackendError>
ScalewayBackend-->>fetch_closure: Result<Option<InstanceSnapshot>, ScalewayBackendError>
fetch_closure-->>poll_for_public_ip: Result<Option<InstanceSnapshot>, ScalewayBackendError>
alt instance running with public_ip
poll_for_public_ip-->>Caller: Ok(InstanceNetworking)
else instance running without public_ip
poll_for_public_ip-->>Caller: Err(ScalewayBackendError::MissingPublicIp)
else never reaches running state before timeout
poll_for_public_ip-->>Caller: Err(ScalewayBackendError::Timeout)
end
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
CodeScene's delta gate flagged the two structurally identical validate_cache_volume_id tests as duplication. A single parameterized case table keeps both assertions (identical trimmed IDs rejected, distinct IDs accepted) without the repeated scaffolding.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
CodeScene's delta gate flagged instance_tags_adds_test_run_tag and volume_tags_adds_test_run_tag as structurally identical (the omits pair had the same shape). Parameterize each pair over the tag-builder function and its base tag, removing the repeated scaffolding without losing any assertion.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
leynos
changed the title
This was referenced Jul 20, 2026
leynos
marked this pull request as ready for review
July 25, 2026 14:53
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6053f54ffb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+100
to
+101
| /// Polls `fetch` until the instance reports a running state with a parseable | ||
| /// public IP, or the timeout elapses. |
There was a problem hiding this comment.
The new documentation for poll_for_public_ip, and likewise poll_until_gone below, describes possible results but provides no example demonstrating invocation and outcome. Add concise examples using a scripted fetch sequence so the functions comply with the repository's mandatory documentation convention.
AGENTS.md reference: AGENTS.md:L27-L30
Useful? React with 👍 / 👎.
| move || ready(Ok(queue.pop_front().unwrap_or(None))) | ||
| } | ||
|
|
||
| fn handle() -> InstanceHandle { |
There was a problem hiding this comment.
In the inspected wait-test module, handle() provides identical setup to four tests but is implemented as an ordinary helper. Convert it to an #[fixture] and inject the handle into those tests, as the repository explicitly requires rstest fixtures for shared setup.
AGENTS.md reference: AGENTS.md:L203-L206
Useful? React with 👍 / 👎.
Review feedback on the mutation-survivor work: - Add `# Examples` sections to `poll_for_public_ip` and `poll_until_gone` showing a scripted fetch sequence and the resulting outcome. Both helpers are crate-internal, so the blocks are `ignore`d per the repository's doctest guide; the executed equivalents live in `super::tests::wait`. - Convert the shared `handle()` helper in the wait tests to an rstest `#[fixture]` injected into the six tests that need it. Also clears the Whitaker `no_expect_outside_tests` findings this surfaced in the sync tests, where helper functions are not recognized as tests: `run_remote_with_fake_output` and `ssh_option_strings` now propagate the config-validation error, and `assert_streaming_runner_output` becomes a macro so it expands inside the calling test and reports that test's line number. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Collaborator
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/scaleway/mod.rs (1)
318-345: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the blank and padded test-run identifiers.
build_tagsat lines 110-121 has three input-dependent behaviours: absent id, blank id after trimming, and trimming of a padded id. These cases drive only the absent path and one already-clean id. A mutant that deletes thetrimmed.is_empty()guard survives. A mutant that removesid.trim()also survives.Add a whitespace-only case to the omission test and a padded case to the addition test.
♻️ Proposed extra cases
#[rstest] -#[case(ScalewayBackend::instance_tags, "ephemeral")] -#[case(ScalewayBackend::volume_tags, "cache")] -fn tags_omit_test_tag_when_unset( +// A blank identifier must be treated as absent, not appended as an empty tag. +#[case(ScalewayBackend::instance_tags, "ephemeral", None)] +#[case(ScalewayBackend::volume_tags, "cache", None)] +#[case(ScalewayBackend::instance_tags, "ephemeral", Some(" "))] +fn tags_omit_test_tag_when_absent_or_blank( #[case] tags_fn: fn(Option<&str>) -> Vec<String>, #[case] base_tag: &str, + #[case] test_run_id: Option<&str>, ) { - let tags = tags_fn(None); + let tags = tags_fn(test_run_id); assert_eq!(tags, vec![String::from("mriya"), String::from(base_tag)]); } #[rstest] -#[case(ScalewayBackend::instance_tags, "ephemeral")] -#[case(ScalewayBackend::volume_tags, "cache")] +#[case(ScalewayBackend::instance_tags, "ephemeral", "run-123")] +#[case(ScalewayBackend::volume_tags, "cache", "run-123")] +// Surrounding whitespace must be trimmed before the tag is built. +#[case(ScalewayBackend::instance_tags, "ephemeral", " run-123 ")] fn tags_add_test_run_tag( #[case] tags_fn: fn(Option<&str>) -> Vec<String>, #[case] base_tag: &str, + #[case] test_run_id: &str, ) { - let tags = tags_fn(Some("run-123")); + let tags = tags_fn(Some(test_run_id)); assert_eq!( tags, vec![ String::from("mriya"), String::from(base_tag), String::from("mriya-test-run-run-123"), ] ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/scaleway/mod.rs` around lines 318 - 345, Add a whitespace-only test case to tags_omit_test_tag_when_unset and assert it returns only the base tags, covering the trimmed-empty identifier path. Update tags_add_test_run_tag to pass a padded identifier and assert the generated test-run tag uses the trimmed value without surrounding whitespace.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/scaleway/lifecycle/tests/mod.rs`:
- Around line 132-147: Merge the duplicate
power_on_if_needed_ignores_non_poweron_actions test into
power_on_if_needed_errors_when_not_allowed by parameterizing the latter with
#[case] values for an empty action list and a single reboot action. Pass the
#[case] allowed Vec<Action> into snapshot, retain the PowerOnNotAllowed
assertion, and remove the standalone duplicate test while preserving the `#58`
exact-match coverage.
In `@src/scaleway/lifecycle/tests/wait.rs`:
- Around line 79-91: Add a test alongside wait_for_public_ip_returns_missing_ip
that scripts absent snapshots and snapshots whose status is not running, then
calls poll_for_public_ip and asserts ScalewayBackendError::Timeout with action
"wait_for_ready". Ensure the sequence exercises both the absent-snapshot branch
and the saw_running == false terminal path.
---
Outside diff comments:
In `@src/scaleway/mod.rs`:
- Around line 318-345: Add a whitespace-only test case to
tags_omit_test_tag_when_unset and assert it returns only the base tags, covering
the trimmed-empty identifier path. Update tags_add_test_run_tag to pass a padded
identifier and assert the generated test-run tag uses the trimmed value without
surrounding whitespace.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 35701ffc-46c7-4d29-9b42-5da8d8676ce0
📒 Files selected for processing (12)
.cargo/mutants.tomlsrc/config_store/tests.rssrc/init/helpers.rssrc/init/mod.rssrc/init/tests.rssrc/scaleway/lifecycle/tests/mod.rssrc/scaleway/lifecycle/tests/wait.rssrc/scaleway/lifecycle/wait.rssrc/scaleway/mod.rssrc/sync/tests/remote.rssrc/sync/tests/ssh.rssrc/sync/tests/streaming.rs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/mapsplice(auto-detected)
Comment on lines
+132
to
+147
| // Kills the `power_on_if_needed` equality-mutant survivor tracked in #58. | ||
| #[rstest] | ||
| #[tokio::test] | ||
| async fn power_on_if_needed_ignores_non_poweron_actions(backend_fixture: ScalewayBackend) { | ||
| // A stopped instance offering only unrelated actions must be reported | ||
| // as not powerable; only an exact "poweron" action may trigger the | ||
| // power-on request. | ||
| let snap = snapshot("id", "stopped", [Action::from("reboot")], None); | ||
| let zone = Zone::from("zone"); | ||
| let result = backend_fixture.power_on_if_needed(&zone, &snap).await; | ||
| assert!( | ||
| matches!(result, Err(ScalewayBackendError::PowerOnNotAllowed { .. })), | ||
| "expected PowerOnNotAllowed error, got {result:?}" | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Merge this test with power_on_if_needed_errors_when_not_allowed.
Lines 132-146 duplicate lines 122-130. The only difference is the allowed-actions argument. Express both inputs as #[case] arguments of one test. The equality-mutant coverage for #58 is preserved, because the "reboot" case still requires an exact poweron match.
As per path instructions: "Replace duplicated tests with #[rstest(...)] parameterised cases."
♻️ Proposed parameterised test
-// Kills the `power_on_if_needed` equality-mutant survivor tracked in `#58`.
-#[rstest]
-#[tokio::test]
-async fn power_on_if_needed_ignores_non_poweron_actions(backend_fixture: ScalewayBackend) {
- // A stopped instance offering only unrelated actions must be reported
- // as not powerable; only an exact "poweron" action may trigger the
- // power-on request.
- let snap = snapshot("id", "stopped", [Action::from("reboot")], None);
- let zone = Zone::from("zone");
- let result = backend_fixture.power_on_if_needed(&zone, &snap).await;
- assert!(
- matches!(result, Err(ScalewayBackendError::PowerOnNotAllowed { .. })),
- "expected PowerOnNotAllowed error, got {result:?}"
- );
-}Replace the earlier test at lines 121-130 with the merged version:
// A stopped instance must be reported as not powerable unless it offers an
// exact "poweron" action. Kills the equality-mutant survivor tracked in `#58`.
#[rstest]
#[case(Vec::<Action>::new())]
#[case(vec![Action::from("reboot")])]
#[tokio::test]
async fn power_on_if_needed_errors_when_not_allowed(
backend_fixture: ScalewayBackend,
#[case] allowed: Vec<Action>,
) {
let snap = snapshot("id", "stopped", allowed, None);
let zone = Zone::from("zone");
let result = backend_fixture.power_on_if_needed(&zone, &snap).await;
assert!(
matches!(result, Err(ScalewayBackendError::PowerOnNotAllowed { .. })),
"expected PowerOnNotAllowed error, got {result:?}"
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Kills the `power_on_if_needed` equality-mutant survivor tracked in #58. | |
| #[rstest] | |
| #[tokio::test] | |
| async fn power_on_if_needed_ignores_non_poweron_actions(backend_fixture: ScalewayBackend) { | |
| // A stopped instance offering only unrelated actions must be reported | |
| // as not powerable; only an exact "poweron" action may trigger the | |
| // power-on request. | |
| let snap = snapshot("id", "stopped", [Action::from("reboot")], None); | |
| let zone = Zone::from("zone"); | |
| let result = backend_fixture.power_on_if_needed(&zone, &snap).await; | |
| assert!( | |
| matches!(result, Err(ScalewayBackendError::PowerOnNotAllowed { .. })), | |
| "expected PowerOnNotAllowed error, got {result:?}" | |
| ); | |
| } | |
| // A stopped instance must be reported as not powerable unless it offers an | |
| // exact "poweron" action. Kills the equality-mutant survivor tracked in `#58`. | |
| #[rstest] | |
| #[case(Vec::<Action>::new())] | |
| #[case(vec![Action::from("reboot")])] | |
| #[tokio::test] | |
| async fn power_on_if_needed_errors_when_not_allowed( | |
| backend_fixture: ScalewayBackend, | |
| #[case] allowed: Vec<Action>, | |
| ) { | |
| let snap = snapshot("id", "stopped", allowed, None); | |
| let zone = Zone::from("zone"); | |
| let result = backend_fixture.power_on_if_needed(&zone, &snap).await; | |
| assert!( | |
| matches!(result, Err(ScalewayBackendError::PowerOnNotAllowed { .. })), | |
| "expected PowerOnNotAllowed error, got {result:?}" | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/scaleway/lifecycle/tests/mod.rs` around lines 132 - 147, Merge the
duplicate power_on_if_needed_ignores_non_poweron_actions test into
power_on_if_needed_errors_when_not_allowed by parameterizing the latter with
#[case] values for an empty action list and a single reboot action. Pass the
#[case] allowed Vec<Action> into snapshot, retain the PowerOnNotAllowed
assertion, and remove the standalone duplicate test while preserving the `#58`
exact-match coverage.
Source: Path instructions
Comment on lines
+79
to
91
| #[rstest] | ||
| #[tokio::test] | ||
| async fn wait_for_public_ip_returns_missing_ip(handle: InstanceHandle) { | ||
| let fetch = scripted_fetch(vec![ | ||
| Some(super::snapshot("id", "running", Vec::<Action>::new(), None)), | ||
| Some(super::snapshot("id", "running", Vec::<Action>::new(), None)), | ||
| ]); | ||
| let result = poll_for_public_ip(&handle, poll_settings(Duration::from_millis(5)), fetch).await; | ||
| assert!( | ||
| matches!(result, Err(ScalewayBackendError::MissingPublicIp { .. })), | ||
| "unexpected wait_for_public_ip outcome: {result:?}" | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add a case for the never-running timeout path.
poll_for_public_ip has two terminal error arms. The tests exercise only MissingPublicIp. Nothing drives saw_running == false, so the ScalewayBackendError::Timeout { action: "wait_for_ready" } arm is unverified. A mutant that sets saw_running = true unconditionally, or that swaps the two terminal errors, survives this suite. The same gap leaves the absent-snapshot branch (let Some(server) = ... else) unexercised.
Add a test that scripts only absent and non-running snapshots.
♻️ Proposed additional test
+#[rstest]
+#[tokio::test]
+async fn wait_for_public_ip_times_out_when_never_running(handle: InstanceHandle) {
+ // An absent instance followed by a non-running one must exhaust the
+ // deadline and report a timeout rather than a missing address.
+ let fetch = scripted_fetch(vec![
+ None,
+ Some(super::snapshot(
+ "id",
+ "starting",
+ Vec::<Action>::new(),
+ None,
+ )),
+ ]);
+ let result = poll_for_public_ip(&handle, poll_settings(Duration::from_millis(5)), fetch).await;
+ assert!(
+ matches!(
+ result,
+ Err(ScalewayBackendError::Timeout { ref action, .. }) if action == "wait_for_ready"
+ ),
+ "unexpected wait_for_public_ip outcome: {result:?}"
+ );
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #[rstest] | |
| #[tokio::test] | |
| async fn wait_for_public_ip_returns_missing_ip(handle: InstanceHandle) { | |
| let fetch = scripted_fetch(vec![ | |
| Some(super::snapshot("id", "running", Vec::<Action>::new(), None)), | |
| Some(super::snapshot("id", "running", Vec::<Action>::new(), None)), | |
| ]); | |
| let result = poll_for_public_ip(&handle, poll_settings(Duration::from_millis(5)), fetch).await; | |
| assert!( | |
| matches!(result, Err(ScalewayBackendError::MissingPublicIp { .. })), | |
| "unexpected wait_for_public_ip outcome: {result:?}" | |
| ); | |
| } | |
| #[rstest] | |
| #[tokio::test] | |
| async fn wait_for_public_ip_returns_missing_ip(handle: InstanceHandle) { | |
| let fetch = scripted_fetch(vec![ | |
| Some(super::snapshot("id", "running", Vec::<Action>::new(), None)), | |
| Some(super::snapshot("id", "running", Vec::<Action>::new(), None)), | |
| ]); | |
| let result = poll_for_public_ip(&handle, poll_settings(Duration::from_millis(5)), fetch).await; | |
| assert!( | |
| matches!(result, Err(ScalewayBackendError::MissingPublicIp { .. })), | |
| "unexpected wait_for_public_ip outcome: {result:?}" | |
| ); | |
| } | |
| #[rstest] | |
| #[tokio::test] | |
| async fn wait_for_public_ip_times_out_when_never_running(handle: InstanceHandle) { | |
| // An absent instance followed by a non-running one must exhaust the | |
| // deadline and report a timeout rather than a missing address. | |
| let fetch = scripted_fetch(vec![ | |
| None, | |
| Some(super::snapshot( | |
| "id", | |
| "starting", | |
| Vec::<Action>::new(), | |
| None, | |
| )), | |
| ]); | |
| let result = poll_for_public_ip(&handle, poll_settings(Duration::from_millis(5)), fetch).await; | |
| assert!( | |
| matches!( | |
| result, | |
| Err(ScalewayBackendError::Timeout { ref action, .. }) if action == "wait_for_ready" | |
| ), | |
| "unexpected wait_for_public_ip outcome: {result:?}" | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/scaleway/lifecycle/tests/wait.rs` around lines 79 - 91, Add a test
alongside wait_for_public_ip_returns_missing_ip that scripts absent snapshots
and snapshots whose status is not running, then calls poll_for_public_ip and
asserts ScalewayBackendError::Timeout with action "wait_for_ready". Ensure the
sequence exercises both the absent-snapshot branch and the saw_running == false
terminal path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #55, #56, #57, #58, #59.
Summary
Kills all 32 mutation-testing survivors from the full dispatch run
(29074663742,
dataset SHA
1004e607): 26 real test gaps closed with new unit tests, onesubsumed predicate clause deleted (two mutants), one equivalent mutant and
three test-backdoor scaffolding functions excluded via a new
.cargo/mutants.tomlwith inline justifications. Scoped confirmationre-runs surfaced four further missed mutants beyond the worklist; these are
also killed. Two new network-boundary mutants remain and are documented
below.
Review walkthrough
src/config_store/tests.rs(Mutation testing: config store current_volume_id survivors #55): the real on-disk
ConfigStore::current_volume_idpath was onlyever observed through a test double. New tests exercise the production
implementation against a temporary directory: a write/read round trip
asserting the exact ID, a missing-config read asserting
Ok(None), and aprobe through a regular file asserting the
NotADirectoryfailuresurfaces as an
Ioerror rather than "does not exist".src/init/tests.rsand
src/init/helpers.rs(Mutation testing: init validation and failure-message survivors #56): unit tests for
InitConfig::validate(zero rejected, non-zeroaccepted), a four-case table test covering every
format_failure_messagearm, plus
FormatFailureDisplay andappend_teardown_notecoverage.The tautological
BYTES_PER_GBtest now asserts the literal2_147_483_648instead of2 * BYTES_PER_GB, which mutated togetherwith the code under test.
src/scaleway/mod.rs(Mutation testing: Scaleway error classification and volume validation survivors #57): direct unit tests for
is_instance_type_error(fourclassification cases) and
validate_cache_volume_id(identical trimmedIDs rejected, distinct IDs accepted). The dead third clause of
is_instance_type_error— subsumed by the first clause, so no mutationinside it could ever change the result — is deleted (category 3, two
mutants).
src/scaleway/lifecycle/wait.rs(Mutation testing: Scaleway lifecycle wait/create survivors #58): the fetch seam. The polling loop bodies move verbatim into
generic
poll_for_public_ip/poll_until_gonehelpers parameterizedover an instance-fetch closure (timing and port grouped in a copyable
PollSettings);wait_for_public_ipandwait_until_gonebecome thindelegating wrappers passing
|| self.fetch_instance(handle). Behaviouris unchanged.
src/scaleway/lifecycle/tests/wait.rs(Mutation testing: Scaleway lifecycle wait/create survivors #58): the
FakeBackendthat duplicated the polling algorithm isdeleted; the tests drive the production loops with scripted snapshot
sequences, plus two new cases: a happy path (
[starting, running + IP]succeeds) and an already-gone teardown (
Ok(())).src/scaleway/lifecycle/tests/mod.rs(Mutation testing: Scaleway lifecycle wait/create survivors #58):
power_on_if_neededwith a stopped snapshot offering only arebootaction must reportPowerOnNotAllowed; the surviving==→!=mutant instead attempts the power-on API call and surfaces a provider
error.
src/sync/tests/ssh.rs(Mutation testing: sync SSH option survivors #59):
common_ssh_optionsassertions for both toggles —StrictHostKeyChecking=nopresent when checking is disabled and absentwhen enabled, and
UserKnownHostsFile=<path>present for a configuredpath and absent for blank or whitespace-only values.
.cargo/mutants.toml:excludes the
cfg(any(test, feature = "test-backdoors"))scaffolding insrc/main.rs(enable_fake_modes,fake_run_from_env,fake_dump_request,prefail_from_env) — mirroring the callerworkflow's existing
src/test_support.rsexclude-glob — and theequivalent
InstanceRequest::builder→Default::default()mutant(
InstanceRequestBuilder::new()is defined asSelf::default(), so themutant is behaviourally identical). Justifications are inline in the
file.
Survivor counts (before → after)
Scoped re-runs:
cargo mutants --file <file> -- --all-featuresat thisbranch's head.
src/config_store/mod.rssrc/init/mod.rssrc/init/helpers.rssrc/scaleway/mod.rssrc/scaleway/lifecycle/wait.rssrc/scaleway/lifecycle/create.rssrc/sync/mod.rssrc/main.rs--listsrc/backend.rs--listFour additional missed mutants beyond the worklist were surfaced by the
scoped re-runs and killed in the same PR: the
path_existsNotFoundmatch guard (config store),
FormatFailure::fmt, and bothappend_teardown_notereplacements (init).Red–green evidence
Each module's new tests were verified by hand-applying a representative
surviving mutant, observing the failure, reverting, and observing the
pass. One transcript per module:
config_store (
mod.rs:139delete!):init (
helpers.rs:3replace*with+inBYTES_PER_GB):scaleway classification (
mod.rs:42replace||with&&):lifecycle wait (
wait.rsreplace!=with==in the state check):lifecycle create (
create.rs:50replace==with!=):sync (
mod.rs:241delete!):Validation
make check-fmt,make lint(doc, clippy, Whitaker dylint), andmake test(--all-targets --all-features, warnings denied) all pass.make markdownlintandmake test-workflow-contractspass.cargo mutants --filere-runs per file as tabulated above.Notes
wait.rs, both at the networkboundary:
fetch_instance -> Ok(None)(the function is a thin mappingover the live Scaleway list-instances call) and the delegation mutant
wait_until_gone -> Ok(())on the thin wrapper introduced by the seam.Killing either would require stubbing the Scaleway HTTP API in unit
tests; they are documented here rather than excluded so future runs
keep reporting them honestly. In the harvested run the former appears
to account for the single timeout (the 300-second production
wait-timeout spun until the harness gave up).
power_on_if_neededkill is observable because the mutant pathattempts a real API call that fails; the un-mutated test path performs
no network I/O.
Summary by Sourcery
Strengthen Scaleway lifecycle, init, config store, and sync SSH behaviour with new unit tests and polling helpers, and configure mutation-testing exclusions for non-production scaffolding and an equivalent builder mutant.
New Features:
Enhancements:
Build:
Documentation:
Tests:
References