test: add panic-fuzz proptest and split test crates 🎲#133
Merged
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…TING 📘
- Add `known_regressions_do_not_panic` (cases=0) so saved panic seeds
are checked on every `cargo test` run; keep `fuzz_random_tokens`
`#[ignore]`d for opt-in fresh fuzzing.
- Add CONTRIBUTING.md with build/test instructions for the three test
crates and the fuzz harness; update CLAUDE.md to match the new
`tests/{functional,compiler,proptest}` layout.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 2-token `[Identifier, LogicNot]` regression hits a real parser panic at ndc_parser/src/parser.rs:175 — pulling it out of the regressions file until the underlying bug is fixed so CI passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6fd69b2fd
ℹ️ 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".
Codex review: silently dropping the receiver on `recv_timeout` left the spawned worker running forever. Repeated hangs accumulated leaked threads until the OS thread limit was hit, after which `spawn worker thread` would panic for infrastructure reasons rather than language bugs — making the harness unreliable in exactly the non-termination scenario it was supposed to tolerate. Convert timeouts into a clear `panic!` with the elapsed budget so proptest shrinks them like any other panic. The worker is still leaked (Rust has no cooperative thread cancellation), but proptest stops generating fresh cases on first failure, so leaks are bounded by `max_shrink_iters` instead of `cases`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
timfennis
added a commit
that referenced
this pull request
May 7, 2026
## Summary The proptest panic fuzzer (added in #133) found that token streams like `1 not` panic the parser. In `consume_binary_expression_left_associative`, when a `not` token was consumed expecting an augmented operator (e.g. `not in`, `not ==`) and the following token was missing, the error-construction path called `require_current_token().expect("there has to be a token")` — which panics on end-of-input. ## Changes - `ndc_parser/src/parser.rs`: replace the panicking `require_current_token().expect(...)` calls with a guarded match: emit `Error::text("unexpected token …")` when a current token exists, otherwise `Error::end_of_input` using the `not` token's span. - `tests/proptest/tests/panic.regressions`: persisted seed that proptest shrunk to `[2] 1 not`, so this case is locked in as a regression. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Summary
Vec<TokenLocation>directly intoParser::from_tokens→ analyser → compiler → VM, looking for panics,unwraps, andunreachable!s. Errors are ignored — only panics fail the test. Each case runs in a worker thread with a 500ms timeout so generated infinite loops don't hang the suite.#[ignore](it already finds real bugs); opt-in viacargo test -p proptest_tests -- --ignored. Tune withPROPTEST_CASES=…. Saved regressions live next to the test inpanic.regressionsand replay automatically.tests/parent:tests/functional/(wastests/),tests/compiler/(wascompiler_tests/),tests/proptest/(new). Each has its own dependencies so proptest's deps don't leak into the existing test binaries — and the temporaryuse foo as _;shims I needed under the old single-crate layout are gone.Notes for reviewers
Programnewtype with customDebugprints[2] a notinstead of three nestedTokenLocationblocks, and a thread-name-targeted panic hook silences the per-shrink-iteration panic flood without hiding the final test-thread summary from proptest.ndc_parser/src/parser.rs:175,expect("there has to be a token")whennotfollows a primary at end-of-input). The seed is checked in viapanic.regressions, so any future fix will be guarded automatically.OpAssignonly wraps augmentable inner tokens, so we don't surface "panics" no real source could produce.🤖 Generated with Claude Code