test(lana): type-check the mock log builders - #1073
Open
lukecotter wants to merge 1 commit into
Open
lukecotter wants to merge 1 commit into
lukecotter wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 PR Overview
Stacked on #1068 — base is
feat-adopt-apex-log-parser, notmain.lana's two mock log factories returned{ ...base, ...overrides } as unknown as ApexLog. Thatcast erased every check, and the adoption PR showed what it costs: a flat
debugLevels: []and apre-reshape
governorLimitssat in the fixtures through the whole parser swap, because nothingcould 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
Partial<Pick<LogEvent, …>>andPartial<Pick<ApexLog, …>>— instead of restating field types by hand. The oldPartialLogEvent.typesaidstring | nullwhere the class saysLogEventType | null, and thatrestatement is what let the bad
categorythrough.satisfieson each base literal, so the defaults are checked too. This is load-bearing: a bareaschecks nothing, so the defaults have to be validated before the assertion, not by it.as unknown ashop is gone. A plainas LogEvent/as ApexLogcompiles.lanaactually reads —childrenonApexLog, eleven fields onLogEvent(
sizestays for the cache tests). Verified against all five production files that import theparser:
LogEventCache.ts,RawLogFoldingProvider.ts,RawLogSymbolProvider.ts,RawLogLineDecoration.ts,log-utils.ts.emptyLimits()— it only ever fedgovernorLimits, which nothing reads — andcreateMockEventTree, which has no callers anywhere in the repo.Net: 166 lines out, 23 in.
🧩 Type of change (check all applicable)
📷 Screenshots / gifs / video [optional]
None — test-only change.
🔗 Related Issues
None.
✅ Tests added?
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:
Both compiled silently before this PR.
📚 Docs updated?
No user-visible change.
Anything else we need to know? [optional]
Verified:
pnpm run typecheckclean · 395 tests in 28 suites pass ·prettier --checkandeslintclean on the changed file.What this does not catch.
Pickandsatisfiescatch the parser package renaming orretyping a field. They do not catch
lanastarting to read a twelfth field — that compiles, andthe mock supplies
undefined. It surfaces as aCannot read properties of undefinedin the testfor that change rather than silently, which is why the residual
asis acceptable.Considered and rejected: building real instances.
new ApexLog(new ApexLogParser())works andwould delete the assertion outright. But all eleven reads are on
LogEvent, andnew MethodEntryLine(parser, parts)needs a hand-writtenpartsarray encoding the parser's linegrammar — trading a coupling the compiler checks (field names) for one nothing checks (line
format). Doing it for
ApexLogalone protects fields no test reads and would needLogEventCache.test.ts's wholesalejest.mockof the parser module relaxed torequireActual.That
jest.mockreplacing the whole module to stub oneparsefunction is over-broad on its ownmerits — noted here, not fixed in this PR.