Skip to content

test(lana): type-check the mock log builders - #1073

Open
lukecotter wants to merge 1 commit into
feat-adopt-apex-log-parserfrom
test-typed-lana-mock-builders
Open

lukecotter wants to merge 1 commit into
feat-adopt-apex-log-parserfrom
test-typed-lana-mock-builders

Conversation

@lukecotter

Copy link
Copy Markdown
Collaborator

📝 PR Overview

Stacked on #1068 — base is feat-adopt-apex-log-parser, not main.

lana's two mock log factories returned { ...base, ...overrides } as unknown as ApexLog. That
cast erased every check, and the adoption PR showed what it costs: a flat debugLevels: [] and a
pre-reshape governorLimits sat in the fixtures through the whole parser swap, because nothing
could see them. A third wrong value was in there too, and predates the swap — category: 'Method',
which is not a member of LogCategory.

This makes the fixtures type-check.

🛠️ Changes made

  • Both override aliases derive from the library class — Partial<Pick<LogEvent, …>> and
    Partial<Pick<ApexLog, …>> — instead of restating field types by hand. The old
    PartialLogEvent.type said string | null where the class says LogEventType | null, and that
    restatement is what let the bad category through.
  • satisfies on each base literal, so the defaults are checked too. This is load-bearing: a bare
    as checks nothing, so the defaults have to be validated before the assertion, not by it.
  • The as unknown as hop is gone. A plain as LogEvent / as ApexLog compiles.
  • Bases cut to what lana actually reads — children on ApexLog, eleven fields on LogEvent
    (size stays for the cache tests). Verified against all five production files that import the
    parser: LogEventCache.ts, RawLogFoldingProvider.ts, RawLogSymbolProvider.ts,
    RawLogLineDecoration.ts, log-utils.ts.
  • Deleted emptyLimits() — it only ever fed governorLimits, which nothing reads — and
    createMockEventTree, which has no callers anywhere in the repo.

Net: 166 lines out, 23 in.

🧩 Type of change (check all applicable)

  • 🐛 Bug fix - something not working as expected
  • ✨ New feature – adds new functionality
  • ♻️ Refactor - internal changes with no user impact
  • ⚡ Performance Improvement
  • 📝 Documentation - README or documentation site changes
  • 🔧 Chore - dev tooling, CI, config
  • 💥 Breaking change

📷 Screenshots / gifs / video [optional]

None — test-only change.

🔗 Related Issues

None.

✅ Tests added?

  • 👍 yes
  • 🙅 no, not needed
  • 🙋 no, I need help

No new tests: this changes the type of the fixtures, not their behaviour. The same 395 tests across
28 suites pass, and the guard is proved by putting the two old values back:

test-builders.ts(40,5): error TS2322: Type '"Method"' is not assignable to type 'LogCategory | undefined'.
test-builders.ts(66,5): error TS2559: Type 'never[]' has no properties in common with type 'DebugLevels'.

Both compiled silently before this PR.

📚 Docs updated?

  • 🔖 README.md
  • 🔖 CHANGELOG.md
  • 📖 help site
  • 🧪 Marked any pre-release-only features
  • 🙅 not needed

No user-visible change.

Anything else we need to know? [optional]

Verified: pnpm run typecheck clean · 395 tests in 28 suites pass · prettier --check and
eslint clean on the changed file.

What this does not catch. Pick and satisfies catch the parser package renaming or
retyping a field. They do not catch lana starting to read a twelfth field — that compiles, and
the mock supplies undefined. It surfaces as a Cannot read properties of undefined in the test
for that change rather than silently, which is why the residual as is acceptable.

Considered and rejected: building real instances. new ApexLog(new ApexLogParser()) works and
would delete the assertion outright. But all eleven reads are on LogEvent, and
new MethodEntryLine(parser, parts) needs a hand-written parts array encoding the parser's line
grammar — trading a coupling the compiler checks (field names) for one nothing checks (line
format). Doing it for ApexLog alone protects fields no test reads and would need
LogEventCache.test.ts's wholesale jest.mock of the parser module relaxed to requireActual.

That jest.mock replacing the whole module to stub one parse function is over-broad on its own
merits — noted here, not fixed in this PR.

The builders returned `as unknown as ApexLog`, which erased all checking.
That is how `category: 'Method'` — not a LogCategory — and a flat
debugLevels array survived the parser swap.

Both override aliases now derive from the class with Pick, so a field the
parser renames or retypes is a compile error rather than a silent lie.

Cut the bases to what lana reads: children on ApexLog, eleven fields on
LogEvent. emptyLimits fed governorLimits, which nothing reads, and
createMockEventTree had no callers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant