test(group): fail loud on CI when tree-sitter-kotlin grammar is unavailable#1869
Open
henry201605 wants to merge 1 commit into
Open
test(group): fail loud on CI when tree-sitter-kotlin grammar is unavailable#1869henry201605 wants to merge 1 commit into
henry201605 wants to merge 1 commit into
Conversation
…ilable Address Claude's review on PR abhigyanpatwari#1855 (Finding 2). The Kotlin Provider suite (PR abhigyanpatwari#1849) and Consumer suite (PR abhigyanpatwari#1855) both gate their `it()` calls behind: const kotlinAvailable = getPluginForFile('Probe.kt') !== undefined; const itKotlin = kotlinAvailable ? it : it.skip; Locally this is intentional — contributors on platforms where the optional `tree-sitter-kotlin` native binding can't build (or who haven't installed it) shouldn't be blocked by red Kotlin tests. On CI it's a silent-skip hazard: if the optional dep ever fails to install (npm registry hiccup, build-toolchain drift on a runner image update, ABI bump in tree-sitter-kotlin) all 17+ Kotlin cases would silently skip and CI would still be green. The exact failure mode flagged in Claude's review on PR abhigyanpatwari#1855 (Finding 2). Fix: a module-load tripwire that throws ONLY when: - process.env.CI is set (local runs are unaffected), AND - getPluginForFile('Probe.kt') returns undefined Mirrors the existing CI tripwire pattern in `test/integration/object-literal-owner-resolution.test.ts:55-66`, which guards against silent skip on missing dist/parse-worker.js. The error message includes: - what's missing (tree-sitter-kotlin grammar) - what would happen otherwise (silent skip of Provider+Consumer) - where to look (optionalDependencies + runner toolchain) - explicit local-vs-CI reassurance Coverage: this single tripwire protects ALL Kotlin tests in this file — 11 itKotlin (Provider, abhigyanpatwari#1849) + 6 itKotlinConsumer (Consumer, abhigyanpatwari#1855) — and any future Kotlin tests added under the same gating pattern. Reverse-validated four states: 1. local + grammar present → 60/60 pass (unchanged) 2. local + grammar absent → 43 pass + 17 skip (unchanged) 3. CI=true + grammar present → 60/60 pass (unchanged) 4. CI=true + grammar ABSENT → tripwire throws with clear msg (the new behavior — was silently green before, now hard red) Local validation: - test/unit/group/http-route-extractor.test.ts: 60/60 ✅ - test/unit/group: 540/540 ✅ - npm run format:check: clean ✅
|
Someone is attempting to deploy a commit to the NexusCore Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
CI Report✅ All checks passed Pipeline Status
Test Results
✅ All 10001 tests passed 4 test(s) skipped — expand for details
Code CoverageTests
📋 View full run · Generated by CI |
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.
Summary
Follow-up to #1855, addressing the deferred Finding 2 from Claude's review (#1855 (comment)).
The Kotlin Provider tests (#1849) and Consumer tests (#1855) both gate their
it()calls behind:Locally this is intentional — contributors on platforms where the optional
tree-sitter-kotlinnative binding can't build (or who haven't installed it) shouldn't be blocked by red Kotlin tests.On CI it's a silent-skip hazard: if the optional dep ever fails to install (npm registry hiccup, build-toolchain drift on a runner image update, ABI bump in
tree-sitter-kotlin) all 17+ Kotlin cases would silently skip and CI would still pass green.Fix
A module-load tripwire that throws only when both conditions hold:
process.env.CIis setgetPluginForFile('Probe.kt')returnsundefinedMirrors the existing CI tripwire pattern at
test/integration/object-literal-owner-resolution.test.ts:55-66, which guards against silent skip on missingdist/parse-worker.js.The error message states what's missing, what would otherwise happen (silent skip of Provider + Consumer), where to look (
optionalDependencies+ runner toolchain), and explicitly reassures contributors that local runs are unaffected.Coverage
This single tripwire protects all Kotlin tests in
http-route-extractor.test.ts:itKotlincases (Provider — feat(group): add Kotlin Spring HTTP route extraction (named + positional) #1849)itKotlinConsumercases (Consumer — feat(group): add Kotlin Spring HTTP consumer extraction #1855)Reverse-validated four states
CI=true+ grammar presentCI=true+ grammar ABSENTThe "grammar absent" cases were exercised locally by temporarily commenting out
Kotlin = _require('tree-sitter-kotlin')inkotlin.tsand running the suite under both env settings.Local validation
http-route-extractor.test.ts— 60 / 60 ✅test/unit/group— 540 / 540 ✅npm run format:check— clean ✅