Repository navigation
Generate port conformance fixtures from River's Go implementation - #1451
Merged
Merged
Conversation
brandur
approved these changes
Oct 6, 2026
brandur
left a comment
Contributor
There was a problem hiding this comment.
LGTM man! Should we merge this, or do you have it still set to draft because you're working on it?
3 of 6 tasks
brandur
added a commit
that referenced
this pull request
Oct 6, 2026
Just noticed this test case was failing intermittently on: #1451 Problem: the test case scheduled a `setImmediate` callback that'd wait for 100 ms and exit the thread. After that handler returned, the test submitted a second task, expecting the thread to crash before accepting it, but it was possible for the second task's message to arrive and be proceed before the `setImmediate` callback ran. Replace the timing-dependent setup with a test handler that replaces the thread's message listener and exits immediately when the next task arrives, guaranteeing the reused thread dies before acknowledging it, and exercising the pool's retry path deterministically. A side benefit is that we kill the 100 ms busy wait.
bgentry
force-pushed
the
bg/conformance-fixtures
branch
from
October 6, 2026 12:38
db624b4 to
864b4af
Compare
brandur
marked this pull request as ready for review
October 6, 2026 16:33
The Rust and JavaScript ports check unique key hashes, cron next run times, snooze counts, job state bits, retry delay bounds, and notification payloads against goldens recorded from River's Go implementation. Those goldens live as hand-copied JSON inside each port, with no generator in the tree, so when Go's behavior changes the copies keep passing against stale values and nothing flags it. Add a nested `github.com/riverqueue/river/conformance` module with a `generatefixtures` command that writes four fixtures to `conformance/testdata` by calling River's own code: `dbunique.UniqueKey` and `uniquestates.UniqueStatesToBitmask` for `unique_keys.json`, robfig/cron's `ParseStandard` for `cron_schedules.json`, the job executor's snooze rule for `snooze_counters.json`, and River's state, metadata key, notification, attempt error, and retry definitions for `protocol_values.json`. The files use the shapes the ports already read, with cron and snooze split apart so each port test reads only what it needs. The fixtures are generated on demand with `make generate/fixtures` rather than committed, and the directory is ignored by Git. Nothing opaque lands in the tree or in diffs, and generation takes about a second for anyone who already has Go to work on River. Port test targets depend on the generator, so they always read what the current Go code produces. Expose the two values the generator needs from internal packages: `jobexecutor.NextSnoozeCount`, which the executor now calls itself, and `retrypolicy.DelayBounds`, which reports the jitter range of the default retry policy and is checked against `NextRetryAt`. The module is nested, like `riverdriver/riverdrivertest`, so none of it ends up in River's module zip, while it can still import River's internal packages through the workspace. It's never tagged.
The conformance module, its generated fixtures, and the Rust and JavaScript ports stay out of River's Go module zip only because their directories carry their own `go.mod`. Dropping one of those files, or adding fixtures somewhere new, would quietly publish test data in the zip every River user downloads. Add a `checkmodzip` command to the conformance module and a `make check/modzip` target that runs it over every module in `go.work`. It selects each module's files with `golang.org/x/mod/zip`, which applies the same rules as the module proxy, and fails if any `testdata` or `fixtures` directory, any JSON file, or anything under `conformance/`, `js/`, or `rust/` would be included. The conformance module itself is skipped because it holds the fixtures and is never published. CI runs the check in the existing `submodule_check` job.
Point the Rust unique key, cron, snooze, and protocol tests at the fixtures River's Go implementation generates into `conformance/testdata` and delete the hand-copied versions under `tests/fixtures`. The tests read the files at runtime and panic with a pointer to `make generate/fixtures` when one is missing, so an ungenerated fixture fails the test instead of skipping it, and builds and lints that don't run tests don't need Go. `test/rust`, `test/rust/postgres`, and `test/rust/sqlite` now depend on `generate/fixtures`, so they always compare Rust against what the current Go code produces. A new `test/rust/conformance` target runs only the library unit tests and `protocol_fixtures`, which hold every fixture check. The Rust test jobs set up Go for the generator, and the Rust workflow also runs when anything under `conformance/` changes. Give every crate an `include` allowlist so only sources, examples, docs, migrations, the README, and the license are published; tests and their fixtures stay in the repository. `riverqueue` currently publishes all of `tests/`. `make check/rust/package` now also fails if any crate archive would contain a `tests`, `fixtures`, or `testdata` path or a JSON file.
Point the cron and snooze counter tests at the fixtures River's Go implementation generates into `conformance/testdata` and delete the hand-copied goldens. Like the Rust tests, they fail with a pointer to `make generate/fixtures` when a fixture is missing instead of skipping. `make test/js` now depends on `generate/fixtures`, and a new `test/js/conformance` target runs only the two fixture test files, which import sources directly and need no build. The JavaScript unit test job sets up Go and generates the fixtures before testing, and the workflow also runs when anything under `conformance/` changes. The package check now also rejects any packed `testdata`, `fixture`, or `golden` path, any `.tsbuildinfo`, and any JSON file other than `package.json` and `migrations/manifest.json`, so test data can't slip into a published tarball.
The Rust and JavaScript workflows are path filtered to their own files, so a Go change that alters a generated fixture, like a new unique key encoding or a different snooze rule, merges without either port's tests seeing the new values. The mismatch only surfaces later, on an unrelated port change. Add a `Conformance` workflow with one job that runs when Go files, `go.mod`, `go.sum`, `go.work`, the SQL queries the generator reads, or the conformance module change. It regenerates the fixtures and runs only `make test/rust/conformance` and `make test/js/conformance`, with Cargo and pnpm caches, instead of either port's full suite or matrix.
At Codex's suggestion, replace superficial notification checks with a test that sends all seven generated payloads through Rust's dispatcher and verifies cancellation, queue wakeups, and leadership signals.
brandur
force-pushed
the
bg/conformance-fixtures
branch
from
October 6, 2026 16:56
864b4af to
35949ea
Compare
brandur
approved these changes
Oct 6, 2026
brandur
added a commit
that referenced
this pull request
Oct 6, 2026
Codex noticed as it was reviewing #1451 that a more exhaustive set of checks are applied to Rust than TypeScript when it comes to conformance. This is probably due to more recent changes in the conformance suite that hadn't yet made their way to TypeScript. Here, do one more pass to bring TypeScript as inline with Rust as we can make it.
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.
The Rust port checks several values against goldens recorded from River's Go implementation: unique key hashes and state bitmasks, cron next run times, snooze counts, job state bits, retry delay bounds, and notification payloads. The JavaScript port checks the cron and snooze ones too. Those goldens are hand-copied JSON files inside each port (
rust/riverqueue/tests/fixtures/andjs/src/testdata/), and nothing in the tree generates them. When Go's behavior changes, the copies don't. The port tests keep passing against the old values, and nothing flags that the ports now disagree with Go.For example, say a fix changes how Go escapes a selected
river:"unique"field name before hashing it. Go's unique keys for those args change, butunique_keys.jsonin the Rust port still holds the old hashes, so Rust's golden test still passes. On a shared database, a job inserted by Go and the same job inserted by Rust now get different unique keys, so both rows are inserted when one should have been rejected as a duplicate. CI stays green the whole time.This adds a nested
github.com/riverqueue/river/conformancemodule with ageneratefixturescommand. It writes four files toconformance/testdata/by calling River's own code:dbunique.UniqueKeyanduniquestates.UniqueStatesToBitmaskforunique_keys.json, therobfig/cronParseStandardthat River documents for periodic jobs forcron_schedules.json, the job executor's snooze rule forsnooze_counters.json, and River's state, metadata key, notification, attempt error, and retry definitions forprotocol_values.json. To make that possible, the executor's snooze increment moves intojobexecutor.NextSnoozeCount, which the executor now calls itself, andretrypolicy.DelayBoundsreports the default retry policy's jitter range. The files use the shapes the ports already read, and cron and snooze are separate files so the JavaScript tests read exactly what they used to.The fixtures are generated on demand and never committed.
conformance/testdata/is gitignored, andmake generate/fixtureswrites it. That keeps opaque generated JSON out of the tree and out of review diffs, and there's no "regenerate and commit" step to forget. Anyone working on River already has Go, and generation takes about a second. Every make target that runs port tests reading the fixtures (test/rust,test/rust/sqlite,test/rust/postgres,test/js) depends ongenerate/fixtures, so those tests always compare against what the current Go code produces. The Rust and JavaScript tests read the files at runtime rather than embedding them, and a missing file fails the test with "missing conformance fixture …; runmake generate/fixtures" instead of skipping it. Reading at runtime also means Rust builds, clippy, and docs don't need the fixtures, so only the jobs that actually run tests set up Go: the Rustrust_versionsandpostgresjobs and the JavaScript unit test job. Both port workflows now also run when anything underconformance/changes.That still leaves the case from the example above: a Go-only change doesn't touch either port's path filters, so neither port's workflow runs and the drift merges anyway. Rather than widening those filters to every Go file, which would run both ports' full matrices on most Go PRs, a new
Conformanceworkflow with a single job closes the gap. It runs only when Go files,go.mod/go.sum/go.work, the SQL queries the generator reads, orconformance/change. It generates the fixtures and runs only the tests that read them, throughmake test/rust/conformance(theriverqueuelibrary unit tests andprotocol_fixtures) andmake test/js/conformance(the two fixture test files, which run from source with no build). Cargo and pnpm are cached, so it should take a couple of minutes on one runner, and it adds no matrix entries.conformance/is its own module, likeriverdriver/riverdrivertest, for two reasons. A nestedgo.modkeeps the generator and generated fixtures out of River's module zip, so users never download them. It's also a real module rather than a stub, so it can import River's internal packages throughgo.work. It's untagged and never released. A second commit addsmake check/modzip, which runs in thesubmodule_checkCI job. It usesgolang.org/x/mod/zipto list the files each workspace module would publish and fails if any of them aretestdataorfixturespaths, JSON files, or anything underconformance/,js/, orrust/.The port commits also make sure test data can't ship in a package. Today the
riverqueuecrate publishes all oftests/, including the fixture JSON, because Cargo includes it by default. Every crate now has anincludeallowlist, soriverqueuegoes from 107 packaged files to 75, andmake check/rust/packagefails if an archive contains atests,fixtures, ortestdatapath or a JSON file. The npm packages already left test data out through theirfileslists, but nothing enforced it. The JavaScript package check now rejects packedtestdata,fixture, orgoldenpaths and any JSON other thanpackage.jsonand the migrations manifest.