Skip to content

harness: tidy build.rs comment + extract Fixture::fail helper - #4367

Merged
Kenny Kerr (kennykerr) merged 1 commit into
masterfrom
copilot/cleanup-code-and-simplify-testing
May 2, 2026
Merged

harness: tidy build.rs comment + extract Fixture::fail helper#4367
Kenny Kerr (kennykerr) merged 1 commit into
masterfrom
copilot/cleanup-code-and-simplify-testing

Conversation

Copilot AI commented May 2, 2026

Copy link
Copy Markdown
Contributor

Low-risk cleanup of seams left in crates/tests/fixtures/harness/ after the recent test-consolidation churn (#4339, #4342, #4344, #4347#4354, #4365). No behaviour change.

  • build.rs

    • Top-of-file comment claimed generated tests dispatched into harness::run_<group>(name) in the lib; in fact build.rs emits crate::run_fixture(group, name) and the dispatcher lives in tests/fixtures.rs. Updated.
    • Removed dead let _ = &mut groups; (and its incorrect "suppress unused warning" comment) — groups is iterated unconditionally on the line above.
  • tests/fixtures.rs

    • Added Fixture::fail(stage, error) and routed 12 of the 14 unwrap_or_else(|e| panic!("[{}/{}] ...: {e}", f.group, f.name)) sites through it. stage: impl Display so format_args!("reader({path})") works without an extra allocation. The two unwrap_or_else(|| panic!(...)) ("expected to fail but succeeded") sites have a different shape and are left alone.
impl Fixture {
    fn fail(&self, stage: impl std::fmt::Display, error: impl std::fmt::Display) -> ! {
        panic!("[{}/{}] {stage}: {error}", self.group, self.name);
    }
}

// before
.unwrap_or_else(|e| panic!("[{}/{}] reader({}): {e}", f.group, f.name, rdl.display()));
// after
.unwrap_or_else(|e| f.fail(format_args!("reader({})", rdl.display()), e));

Verified that doc/CI sweep items from the plan (stale references to retired docs/{test,cross}-todo.md, linux.yml, tests/roundtrip/*, pre-rename Generic<T>/Ref) are already clean — no edits needed.

Out of scope, deferred to focused follow-ups: lifting the cfg(windows) gate on compile_fixtures.rs, replacing the hand-rolled fixture.toml parser with a real TOML dep, reshaping kind = "reader_no_input", and the msrv.yml / no_std.yml / slim_errors.yml workflow merges.

Copilot AI left a comment

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.

Pull request overview

Cleans up the test-fixtures harness by correcting build.rs documentation and centralizing fixture failure formatting in a Fixture::fail helper, keeping panic output consistent across group runners.

Changes:

  • Update build.rs top-level comment to reflect that generated tests call crate::run_fixture(group, name) and remove a dead unused-variable suppression.
  • Add Fixture::fail(stage, error) and route most framed unwrap_or_else(... panic!(...)) sites through it for uniform error messages.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
crates/tests/fixtures/harness/tests/fixtures.rs Adds Fixture::fail and refactors runner error handling to use it for consistent [group/name] stage: error panics.
crates/tests/fixtures/harness/build.rs Fixes misleading comment about generated test dispatch and removes unnecessary groups mutability/unused suppression.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kennykerr
Kenny Kerr (kennykerr) merged commit 16358b3 into master May 2, 2026
34 checks passed
@kennykerr
Kenny Kerr (kennykerr) deleted the copilot/cleanup-code-and-simplify-testing branch May 2, 2026 11:47
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.

3 participants