Skip to content

Fix: 2 issues (#912, #913) - #936

Open
euniceamoni wants to merge 1 commit into
SorobanGuard:mainfrom
euniceamoni:simple/912-913
Open

euniceamoni wants to merge 1 commit into
SorobanGuard:mainfrom
euniceamoni:simple/912-913

Conversation

@euniceamoni

Copy link
Copy Markdown

Closes #912
Closes #913

Issues

#912 tests: add proptest coverage for the suppression-comment parser

Problem

The // soroban-guard: allow(...) parser has been the source of several bugs (#‍409 fn matched inside strings/comments, #‍602 whitespace-exact prefix). It is covered only by hand-written examples, so more edge cases are likely.

Proposal

Add proptest as a dev-dependency of the analyzer and test properties such as:

  • Parsing arbitrary strings never panics.
  • A generated valid annotation (random whitespace, case, comma-separated check names) always parses to exactly the expected set of names.
  • Adding a suppression for check A never suppresses a finding for check B.

Acceptance criteria

  • Tests run in under ~10s in CI with a fixed case count; failing seeds are persisted via proptest-regressions/.
  • Any bug found is filed or fixed separately and referenced.

#913 ci: add a scheduled mutation-testing job with cargo-mutants for the checks crate

Problem

Several checks shipped with tests that could not fail (#‍528 tautology assertion) or no negative cases (#‍529). Line coverage will not catch that; mutation testing does.

Proposal

Add a weekly (plus manual workflow_dispatch) workflow that runs cargo mutants -p soroban-guard-checks and uploads mutants.out as an artifact. It must not gate PRs. Add a short section to CONTRIBUTING.md on running it locally for a single file (--file crates/checks/src/<check>.rs).

Acceptance criteria

  • Workflow uses a pinned action/tool version and has a timeout.
  • Job summary lists the count of missed mutants.
  • First run's notable survivors are listed in the PR description.

Summary

The suppression-comment parser currently lives in crates/checks/src/annotations.rs, which in this snapshot contains the missing-contract-annotation check; confirm the actual parser location before adding tests and adjust the file path if it has moved. Keep proptest cases fixed at ~256 to meet the ~10s CI budget, and commit any generated proptest-regressions/ files. The mutation workflow must not be added to branch protection or PR triggers; pin the cargo-mutants version explicitly and keep the job summary step running even when mutants are found (use if: always()).

Changes

  • Add proptest = "1" under [dev-dependencies] (alongside the existing walkdir) so the new property tests for the suppression-comment parser can compile. Keep the dependency pinned to the 1.x series. (crates/checks/Cargo.toml)
  • Add a #[cfg(test)] mod proptest_tests (or extend the existing test module) in the suppression-comment parser file. Use ProptestConfig { cases: 256, .. } so the suite stays under ~10s. Implement three properties: (1) parsing arbitrary String input never panics; (2) a strategy that generates a valid // soroban-guard: allow(...) annotation with random whitespace, case, and comma-separated check names, asserting the parsed set equals exactly the expected set; (3) suppressing check A never suppresses a finding for check B. Reference any discovered bugs (e.g. function_name_from_line finds "fn " anywhere in a line — comments, strings, and type positions register bogus suppressions #409, analyzer: the suppression comment prefix is whitespace-exact and check names are never validated — //soroban-guard: allow(x) and typos silently do nothing #602) in comments and file them separately if found. (crates/checks/src/annotations.rs)
  • Add a new integration test file crates/checks/tests/annotations_proptest.rs that exercises the public suppression-comment parsing API with proptest: arbitrary-string no-panic, generated valid annotation round-trip to the expected name set, and non-interference between suppressions for different checks. Use a fixed ProptestConfig { cases: 256, .. } and rely on the default proptest-regressions/ persistence so failing seeds are committed. (crates/checks/tests/annotations_proptest.rs)
  • Ensure proptest-regressions/ directories are not ignored so failing seeds are persisted and committed. The current .gitignore does not exclude them; add an explicit comment/negation (e.g. !**/proptest-regressions/) only if a broader pattern would otherwise hide them, and verify target/ remains ignored. (.gitignore)
  • Add a new workflow .github/workflows/mutants.yml triggered only by schedule (weekly cron) and workflow_dispatch, never on pull_request, so it does not gate PRs. Set timeout-minutes (e.g. 60), check out the repo, install a pinned cargo-mutants version (e.g. cargo install cargo-mutants --version <pinned> --locked), run cargo mutants -p soroban-guard-checks, upload mutants.out via actions/upload-artifact, and append the count of missed mutants to $GITHUB_STEP_SUMMARY. Use if: always() for artifact upload and summary steps. (.github/workflows/mutants.yml)
  • Add a short "Mutation testing" section documenting how to run cargo mutants -p soroban-guard-checks locally and how to target a single check file with --file crates/checks/src/<check>.rs. Mention that the scheduled workflow is non-gating and that notable first-run survivors should be listed in the PR description. (CONTRIBUTING.md)

Co-authored-by: Simple Agent <agent@simple.dev>
@drips-wave

drips-wave Bot commented Oct 22, 2026

Copy link
Copy Markdown

@euniceamoni Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

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.

ci: add a scheduled mutation-testing job with cargo-mutants for the checks crate tests: add proptest coverage for the suppression-comment parser

1 participant