Tests that keep passing when the code they cover is broken
Personne n'a encore pris cette issue.
Évaluation
- Difficulté
- 5/5
- Temps estimé
- Plus d'une semaine
- Accessibilité débutants
- 35/100
- Type d'issue
- Fonctionnalité
- Clarté
- À clarifier
- Activité
- Calme
- Stack technique
- bash, react, typescript
- Domaine
- testing-qa
Piste de recherche
Commencez par test/auth.test.tsx et les instances liées à #779, #782, #785 et #766 ; exécutez les tests concernés et examinez isolément les mutations délibérées décrites. Le travail est considéré comme terminé lorsqu’une convention convenue pour valider les tests qui protègent contre les échecs, y compris les vérifications de mutations isolées, est documentée, ou lorsque la question de savoir si Stryker mérite d’être évalué est consignée.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Description
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 <AuthCheck> 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
initialDatabranch ofgetServerSnapshotwas dead under test. Neuter it to always returnloadingand all 22 tests still passed. Found by Armando on #779, which is still open. does not show a logged-out user after navigating awaysits indescribe('useUser')but stopped callinguseUserwhen #782 replaced its wrapper. MakinguseUserthrow left it passing on that branch while the same mutation failed it onv5. Fixed in #782.- The packed-artifact load check only exercised
import(). Breaking therequire()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.
- Langage dominant
- TypeScript
- Étoiles
- 3.6k
- Forks
- 403
- Merge moyen
- 5 j 1 h
- PR mergées (30 j)
- 10
Guide de contribution
Ouvrir le guide de contribution
Par où commencer
- Lisez l'issue en entier, puis le guide de contribution du projet.
- Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
- Forkez le dépôt et travaillez sur une branche.
- Ouvrez une pull request qui référence le numéro de l'issue.
Autres issues de FirebaseExtended/reactfire
-
Difficulté 5/5 Plus d'une semaine Accessibilité débutants 38/100
FirebaseExtended/reactfire#801 ·
-
v5
Difficulté 4/5 3-5 jours Accessibilité débutants 48/100
FirebaseExtended/reactfire#793 ·
-
v5
Difficulté 4/5 3-5 jours Accessibilité débutants 35/100
FirebaseExtended/reactfire#790 · 2 commentaires ·
-
v5
Difficulté 4/5 3-5 jours Accessibilité débutants 55/100
FirebaseExtended/reactfire#789 ·
-
Difficulté 4/5 3-5 jours Accessibilité débutants 55/100
FirebaseExtended/reactfire#784 ·
Toutes les issues de FirebaseExtended/reactfire
Issues similaires
-
bug(cli): hapi doctor inline-media prints a fabricated B:\ helper-script path in packaged installs Ouverte
Difficulté 2/5 1-3 heures Accessibilité débutants 70/100
-
Crush Ouverte
Difficulté 1/5 Moins d'une heure Accessibilité débutants 85/100
catppuccin/catppuccin#3125 ·
-
Add a SECURITY.md Ouverte
Difficulté 1/5 Moins d'une heure Accessibilité débutants 90/100
ElementsProject/cln-application#167 · 1 commentaire · 1 réaction ·
-
Difficulté 2/5 1-3 heures Accessibilité débutants 75/100
Quantco/pnpm-licenses#17 ·
-
Difficulté 2/5 1-3 heures Accessibilité débutants 75/100