FirebaseExtended / FirebaseExtended/reactfire
Tests that keep passing when the code they cover is broken
- 主要语言
- TypeScript
- 星标
- 3.6k
- 派生
- 403
- 平均合并
- 14 小时 53 分钟
- 30 天内合并 PR
- 5
描述
Several checks in this repo have turned out to pass whether or not the thing they name works. They were found one at a time during review, so this issue collects them and proposes a convention.
A test that asserts nothing is worse than a missing one, because it reads as coverage.
## Instances, and who verified each
**The code never executes.** In `test/auth.test.tsx`, a test rendered `` while `beforeEach` had signed the user out, so the fallback rendered, `UserDetails` never mounted, and its two `expect` calls never ran. Armando re-confirmed on `v5` by putting an assertion that cannot pass inside `UserDetails`: the suite still went green. Fixed in #782.
**The test passes when the thing it names is broken.** Three, all confirmed by mutation:
- The `initialData` branch of `getServerSnapshot` was dead under test. Neuter it to always return `loading` and all 22 tests still passed. Found by Armando on #779, which is still open.
- `does not show a logged-out user after navigating away` sits in `describe('useUser')` but stopped calling `useUser` when #782 replaced its wrapper. Making `useUser` throw left it passing on that branch while the same mutation failed it on `v5`. Fixed in #782.
- The packed-artifact load check only exercised `import()`. Breaking the `require()` half deliberately left every test green. Found on #766. That code has since been removed from the PR, so this one never landed.
**The harness cannot report a failure at all.** The flake probe ran its test command under `bash -e` without a `set +e` guard, so the first failing iteration killed the step before the result was recorded. It could only ever produce a clean table, and the first run that genuinely reproduced the flake would have reported least. Fixed in #785.
**The test catches a mutation for the wrong reason.** On the `startWithValue` removal branch, a test appeared to catch a deliberate break but passed under that same mutation when run in isolation: the failure came from another test's warning in the shared suite. It would have surfaced as an order-dependent CI flake.
## What would catch these
A convention rather than a framework: **any test whose purpose is to guard a specific failure should be shown to fail against a deliberate break of that failure, and the PR should say so.** That is what caught four of the five above, and it costs one run.
Two caveats worth stating. Mutating in the suite is not enough on its own, since coupling between tests can produce the failure for an unrelated reason, so run the mutation in isolation as well. And this only covers tests written deliberately as guards; it says nothing about coverage that quietly evaporates when a wrapper changes, which is what happened in #782.
If we want a tool rather than a convention, mutation testing is off-the-shelf for TS (Stryker), and that is worth pricing before writing anything bespoke.
贡献指南
调研方向
Start with test/auth.test.tsx and the instances linked to #779, #782, #785, and #766; run the relevant tests and review the described deliberate mutations in isolation. Done means documenting an agreed convention for validating failure-guarding tests, including isolated mutation checks, or recording whether Stryker is worth evaluating.
由索引模型根据 Issue 内容生成。
评估
- 技术栈
- bash, react, typescript
- 领域
- testing-qa
- Issue 类型
- 功能
- 难度
- 5/5
- 预计耗时
- 一周以上
- 活跃度
- 冷清
- 描述清晰度
- 需要澄清
- 新手友好度
- 35/100