-
Notifications
You must be signed in to change notification settings - Fork 1
[issue-12] docs: catalog new scanner codes + fix coverage table (#12) #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,9 @@ These mirror the [Python catalog](python.md) entries; the signal is the JavaScri | |
| | C6 | LOW | weak check (`toBeTruthy`/`toBeDefined`, `.length > 0`) | | ||
| | C7 | HIGH | self-compare (`expect(x).toBe(x)`) | | ||
| | C8 | LOW | exact equality on a float | | ||
| | C8b | LOW | `toBeCloseTo` with no precision argument — the default 2-digit tolerance may be too loose | | ||
| | C9 | LOW | `toThrow()` with no error type or message | | ||
| | C11a | LOW | self-confirming literal — the expected value is bound from the same call under test (`const e = foo(); expect(foo()).toBe(e)`) | | ||
| | C16 | LOW | depends on `Date.now` / `new Date()` / `Math.random` / `crypto.randomUUID`/`getRandomValues` / a fixed timer | | ||
| | C18 | LOW | stringified equality (`String(x)` / `JSON.stringify` / template literal) | | ||
| | C20 | HIGH | dead assertion after `return` / `throw` / `process.exit` / an exhaustive `switch` (structured block-level reachability, cfg.ts) | | ||
|
|
@@ -220,6 +222,88 @@ A Cypress query chain (`cy.get`/`cy.find`/`cy.contains`) used as a statement wit | |
| never asserted, the cy.* analogue of JS13. Action commands (`click`/`type`/`visit`/...) do work | ||
| rather than query, so a chain ending in one stays clean, as does one ending in `.should`/`.and`. | ||
|
|
||
| ### JS25 - the only assertion is inside an array-iterator callback | ||
| `J1` · HIGH · F2 | ||
|
|
||
| The sole `expect` sits inside a `forEach` / `map` / `filter` / `some` / `every` / `flatMap` | ||
| callback. On an empty collection the callback never runs, so the test passes having asserted | ||
| nothing. Assert the length first, or pull at least one check outside the iterator. | ||
|
|
||
| === "BAD" | ||
| ```javascript | ||
| it('all valid', () => { | ||
| rows.forEach(r => expect(r.valid).toBe(true)); // JS25 - nothing runs if rows is [] | ||
| }); | ||
| ``` | ||
| === "CLEAN" | ||
| ```javascript | ||
| it('all valid', () => { | ||
| expect(rows.length).toBeGreaterThan(0); | ||
| rows.forEach(r => expect(r.valid).toBe(true)); | ||
| }); | ||
| ``` | ||
|
|
||
| ### JS26 - fake timers installed but never advanced | ||
| `J1` · LOW · F2 | ||
|
|
||
| `jest.useFakeTimers()` / `vi.useFakeTimers()` (or `sinon` fake timers) is set up, but the clock is | ||
| never advanced (`runAllTimers`, `advanceTimersByTime`, `tick`). The scheduled callback never fires, | ||
| so the assertion reads un-mutated state and passes for the wrong reason. | ||
|
|
||
| ### JS27 - toHaveBeenCalled* is the sole oracle on a locally-created double | ||
| `J3` · LOW · F4 | ||
|
|
||
| The only assertion is `toHaveBeenCalled` / `toHaveBeenCalledWith` / `toHaveBeenCalledTimes` on a | ||
| mock created in the test (`jest.fn()` / `vi.fn()`). It verifies the test's own wiring, that the | ||
| double was invoked, not that the unit produced the right result. | ||
|
|
||
| ### JS29 - resolves / rejects chain is a bare statement | ||
| `J1` · LOW · F2 | ||
|
|
||
| `expect(p).resolves.toBe(...)` / `.rejects...` written as a statement that is neither `await`ed | ||
| nor `return`ed. The matcher returns a promise; the test finishes green before it settles, so a | ||
| later rejection is lost. | ||
|
|
||
| === "BAD" | ||
| ```javascript | ||
| it('resolves', () => { | ||
| expect(load()).resolves.toBe(42); // JS29 - not awaited or returned | ||
| }); | ||
| ``` | ||
| === "CLEAN" | ||
| ```javascript | ||
| it('resolves', async () => { | ||
| await expect(load()).resolves.toBe(42); | ||
| }); | ||
| ``` | ||
|
|
||
| ### JS30 - literal-vs-literal assertion | ||
| `J2` · HIGH · F3 | ||
|
|
||
| Both operands are fixed at parse time: `expect(2).toBe(3)`, chai `expect(1).to.equal(1)`. The | ||
| assertion does not touch the unit under test, so it is either always-true or always-false by | ||
| construction, never a check on real behaviour. | ||
|
Comment on lines
+283
to
+285
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
With the documented Useful? React with 👍 / 👎. |
||
|
|
||
| ### JS31 - try/catch swallows a possible throw with no assertion on the exception | ||
| `J1` · LOW · F2 | ||
|
|
||
| A `try` calls the unit and the `catch` neither re-throws nor asserts anything on the error. A unit | ||
| that stops throwing still passes green. Sibling of JS11, where the swallowed thing is an `expect`; | ||
| here there is no assertion at all on the caught path. | ||
|
|
||
| === "BAD" | ||
| ```javascript | ||
| it('throws on bad input', () => { | ||
| try { parse('bad'); } catch (e) { /* JS31 - swallowed, nothing asserted */ } | ||
| }); | ||
| ``` | ||
| === "CLEAN" | ||
| ```javascript | ||
| it('throws on bad input', () => { | ||
| expect(() => parse('bad')).toThrow(SyntaxError); | ||
| }); | ||
| ``` | ||
|
|
||
| ## High-value traps with evidence | ||
|
|
||
| The scanner also catches a set of idiom-specific false-greens documented in the JS/TS empirical | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -303,6 +303,20 @@ confirms Python's attribute assignment works, not the production code. | |
| assert product.price == 100 # C11a - just confirms assignment | ||
| ``` | ||
|
|
||
| ### C52 - membership self-confirmation | ||
| `J2` · LOW · F3 | ||
|
|
||
| `assert x in {x}` (or `x in [x]`, `x in (x,)`): the collection is built from the subject under | ||
| test, so membership holds by construction. A membership variant of C7. Checking against a | ||
| collection assembled independently of the subject is a real check. | ||
|
|
||
| === "BAD" | ||
| ```python | ||
| def test_tag(): | ||
| tag = get_tag() | ||
| assert tag in {tag} # C52 - true by construction | ||
| ``` | ||
|
|
||
| ### C13 - mock assertion misspelled or not called | ||
| `J4` · HIGH · F2 | ||
|
|
||
|
|
@@ -400,6 +414,13 @@ never runs, so the test may be checking a different line than intended. | |
| sut.process(data) # C19 - intended target | ||
| ``` | ||
|
|
||
| ### C49 - pytest.warns / assertWarns wraps more than one call | ||
| `J1` · LOW · F4 | ||
|
|
||
| A `with pytest.warns(W):` / `assertWarns` / `deprecated_call()` block holds more than one | ||
| statement. An unrelated earlier line may emit the warning while the target never does, so the | ||
| test passes without exercising the warning under test. The warns sibling of C19. | ||
|
|
||
| ### C28 - pytest.raises binding variable never read | ||
| `J4` · LOW · F4 | ||
|
|
||
|
|
@@ -418,12 +439,32 @@ checked but not its message or attributes. | |
| assert "must be positive" in str(exc.value) | ||
| ``` | ||
|
|
||
| ### C51 - empty-bodied pytest.raises / warns context | ||
| `J1` · HIGH · F1 | ||
|
|
||
| `with pytest.raises(E):` (or `pytest.warns`) whose body is empty (`pass`, `...`, a comment). | ||
| No call is made inside the block, so the call that should raise never runs and the context | ||
| manager has nothing to catch. Always green. | ||
|
Comment on lines
+445
to
+447
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For the empty Useful? React with 👍 / 👎. |
||
|
|
||
| === "BAD" | ||
| ```python | ||
| with pytest.raises(ValueError): | ||
| pass # C51 - nothing called, nothing raised | ||
| ``` | ||
|
|
||
| ### C29 - os.environ modified directly in a test | ||
| `J6` · LOW · F6 | ||
|
|
||
| `os.environ["KEY"] = value`, `os.environ.update(...)`, or `os.putenv(...)` in a test body. The | ||
| change persists across tests in the same process. Use `monkeypatch.setenv()`. | ||
|
|
||
| ### C55 - comparison between two mock-rooted values | ||
| `J3` · LOW · F4 | ||
|
|
||
| `assert m.foo == m.bar` where both operands derive from the same test double (a `Mock`, | ||
| `MagicMock`, or a `patch`-injected object). Each side is the test's own configured value, so | ||
| the comparison checks the doubles against each other, not the SUT. | ||
|
|
||
| --- | ||
|
|
||
| ## Family D - the test depends on external or shared state | ||
|
|
@@ -511,6 +552,26 @@ capture ran but nothing was checked. | |
| assert out == "hello\n" | ||
| ``` | ||
|
|
||
| ### C50 - caplog / assertLogs output captured but never asserted | ||
| `J4` · LOW · F1 | ||
|
|
||
| `caplog` is read (`caplog.records`, `caplog.text`) or `self.assertLogs(...)` is entered, but the | ||
| captured output is never asserted: no comparison on the records, messages, or levels. The capture | ||
| ran and had no effect on pass/fail. The logging sibling of C31. | ||
|
Comment on lines
+558
to
+560
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When this pattern is Useful? React with 👍 / 👎. |
||
|
|
||
| === "BAD" | ||
| ```python | ||
| def test_logs(caplog): | ||
| run() | ||
| caplog.records # C50 - captured but never asserted | ||
| ``` | ||
| === "CLEAN" | ||
| ```python | ||
| def test_logs(caplog): | ||
| run() | ||
| assert "started" in caplog.text | ||
| ``` | ||
|
|
||
| ### C32 - @pytest.mark.skip without reason | ||
| `J1` · LOW · F5 | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the locally-created mock is passed into the unit as a callback or dependency,
toHaveBeenCalledWith/toHaveBeenCalledTimescan be the observable contract (for example, verifying that the unit calls a notifier with the right payload). Describing every sole call assertion onjest.fn()/vi.fn()as F4 will warn on legitimate interaction tests unless the rule is limited to mocks of the unit under test or doubles invoked only by the test itself.Useful? React with 👍 / 👎.