mirror of
https://github.com/block/buzz.git
synced 2026-11-14 13:21:02 +00:00
test(mobile): run push tests without a gateway dart-define (#9423)
A plain `cd mobile && flutter test` failed 11 push and notification tests on a clean checkout of main, while CI passed. Agents running the full suite kept reporting them as "pre-existing, also fails on main", which hides real regressions in the same files. ### Root cause Push is gated on the compile-time `BUZZ_PUSH_GATEWAY_URL` define (`Env.pushGatewayConfigured`, since [#7261 configurable gateway](https://github.com/block/buzz/pull/7261) made the gateway optional). Without it the app is a push-less build: `readBuzzPushEndpointGrants()` returns no grants and `buzzPushLifecycleEnabled()` is false. CI's mobile shards, `just mobile-test` and the lefthook pre-push lane (which runs `just mobile-test`) all pass `--dart-define=BUZZ_PUSH_GATEWAY_URL=https://push.example`; a bare `flutter test` passes nothing. Each of the 11 was checked against that: - `notification_toggle_test` (5 cases) and `push_lease_revocation_outbox_test` "hosted relay assertion": no endpoint grants without a gateway (`Bad state: No element` at the grant lookup, `No endpoint grant exists for push lease tombstone`). - `push_bridge_test` "reads and exposes persisted endpoint grants" and "debug enrollment carries the configured relay and gateway URLs": the bridge returns `[]` before calling native (`Expected: an object with length of <1>, Actual: []`). - `push_bootstrap_test` "default push requires relay capability" and "new community starts registration": the lifecycle is disabled, so registration never starts (`Expected <1>, Actual <0>`). - `push_bridge_test` "development push gateway matches the compiled configuration": a stale test. It expected a `https://push.buzz.xyz` default that [#7261 configurable gateway](https://github.com/block/buzz/pull/7261) removed from the app; under CI's define both sides equal `push.example`, so it could never fail there. The same files pass on the same Mac with the define, so nothing is environmental. The `Unexpected native call takePendingNotificationResponse` log is unrelated noise: the toggle test's native mock didn't answer that call, which logs (and is caught) in CI too. ### Fix Tests get the gateway from a test hook instead of the define. `Env.pushGatewayUrl` becomes a getter over the compiled value with a `@visibleForTesting` `debugPushGatewayUrlOverride` (the same pattern as `debugUriStreamOverride`), and `test/flutter_test_config.dart` sets it for every test file. `push_unconfigured_build_test` sets it to `''`, so it runs in the main suite instead of being skipped there and run separately. CI, the Justfile, `scripts/flake-check.mjs` and the AGENTS/README check commands drop the define, so CI, `just mobile-test`, pre-push and a bare `flutter test` run the same thing. I chose this over documenting a canonical command because the define can't be made automatic: `flutter test` has no config file for defines, so any doc fix still leaves the obvious command red. A test hook makes every invocation agree. The stale test is replaced by one in the push-less file that binds the production read: with the override cleared, `Env.pushGatewayUrl` must equal the compiled `BUZZ_PUSH_GATEWAY_URL`. Now that CI compiles without the define, it fails if the app regains a built-in gateway default, which the old test never could. The toggle test's mock answers `takePendingNotificationResponse` with no pending response. What gets better: one test mode everywhere, and the push-less tests run in every run. Nothing gets worse: production reads the same compiled value (the override is never set outside tests, and an `aot-snapshot` probe shows AOT still folds the getter to the compiled constant and drops push-only code when the define is absent), and every test still runs in the push-capable mode CI used before. ### Validation - On a fresh origin/main worktree with the Hermit toolchain, a bare `flutter test` failed exactly these 11; the same files passed on the same machine with the define. - After the fix, the pre-push `just mobile-check` and `just mobile-test` (now a bare `flutter test` plus the recipe and secp256k1 checks) pass, with the push-less tests running instead of skipped. `scripts/flake-check.test.mjs` passes. - Falsifiability: re-adding a `defaultValue` to the compiled gateway fails the new no-default test, and removing `flutter_test_config.dart` brings back the other 10 failures. - Unrelated: an earlier full run also failed `community_provider_test` "discard orphan snapshots and image caches", which waits 500 ms for a background directory delete. The Mac was running other full suites at the time; the file passed 8 of 8 runs alone, with and without the define. It is not touched here. Signed-off-by: Tom Brow <tomb@squareup.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -91,9 +91,6 @@ export const nodeTestArgs = (file) =>
|
||||
"--test",
|
||||
]
|
||||
: ["--import", "./test-loader.mjs", "--experimental-strip-types", "--test"];
|
||||
// Mirrors the mobile test shards in .github/workflows/_ci-clients.yml.
|
||||
export const dartDefine =
|
||||
"--dart-define=BUZZ_PUSH_GATEWAY_URL=https://push.example";
|
||||
// Mirrors the postgres-ci default-filter in .config/nextest.toml
|
||||
// (`test(/postgres_tests::/)`, `binary(/^postgres_/)` and
|
||||
// `test(/(^|::)external_infra[^:]*::/)`): those tests need Postgres, Redis,
|
||||
@@ -1163,7 +1160,6 @@ function repeatDart(files, iterations) {
|
||||
"flutter",
|
||||
[
|
||||
"test",
|
||||
dartDefine,
|
||||
`--file-reporter=json:${reportFile}`,
|
||||
...files.map((file) => file.replace(/^mobile\//, "")),
|
||||
],
|
||||
|
||||
@@ -17,7 +17,6 @@ import {
|
||||
chooseRust,
|
||||
classify,
|
||||
cratePackage,
|
||||
dartDefine,
|
||||
hunkRanges,
|
||||
ITERATIONS,
|
||||
infraTest,
|
||||
@@ -1028,16 +1027,11 @@ test("reports give per-test statuses per iteration", () => {
|
||||
});
|
||||
|
||||
test("repeat invocations mirror the lanes they stand in for", () => {
|
||||
// node:test flags come from desktop/package.json "test"; the Flutter define
|
||||
// from the mobile test shards; nextest's flake profile must keep retries off.
|
||||
// node:test flags come from desktop/package.json "test"; nextest's flake
|
||||
// profile must keep retries off.
|
||||
const script = JSON.parse(read("../desktop/package.json")).scripts.test;
|
||||
for (const file of ["x.test.mjs", "x.jsdom-test.mjs"])
|
||||
assert.ok(script.includes(`node ${nodeTestArgs(file).join(" ")} "`), file);
|
||||
assert.ok(
|
||||
read("../.github/workflows/_ci-clients.yml").includes(
|
||||
`scripts/mobile-test-shard.sh \${{ matrix.shard }} 3 ${dartDefine}`,
|
||||
),
|
||||
);
|
||||
// nextest reads its config from the workspace root, so each Rust workspace
|
||||
// carries the flake profile.
|
||||
for (const config of [
|
||||
|
||||
Reference in New Issue
Block a user