fix(ci): migrate coverage to c8 and fix non-TTY progressBar crash#179
Merged
Conversation
Cross-Platform Tests have failed on main since the ESM modernization in #177. The coverage step ("Generate coverage report") failed for two independent reasons: 1. nyc reported 0% coverage across all files. nyc's instrumenter relied on the CommonJS-interop shims that #177 removed; with native ESM it captures nothing. Migrate coverage from nyc to c8 (V8-native, ESM-aware), which captures real coverage. .nycrc.json -> .c8rc.json (same include/exclude/ thresholds); the json reporter is added so codecov still gets coverage/coverage-final.json. 2. The actual exit-1 cause: the terminal-kit progressBar test threw "RangeError: Invalid count value: Infinity". terminal-kit queries the cursor location, which returns Infinity when stdout is not an interactive TTY (CI, piped, redirected), then calls String.repeat(Infinity). Guard loadRealTerminalSync() to fall back to the console stub in non-TTY contexts. This also fixes real piped CLI usage (e.g. hana-cli ... > log). Verified locally: coverage:ci runs clean (exit 0), 555 passing / 0 failing (progressBar test now passes), c8 reports real coverage (was 0%), and coverage/coverage-final.json is produced for the codecov upload.
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.
Problem
Cross-Platform Tests have been failing on
mainsince PR #177 ("refactor: ESM modernization — retire CommonJS-interop shims"). The last green run was #167; every run since — the dependabot bumps and PR #178 — inherited the failure.The failing step is "Generate coverage report" (
npm run coverage:ci), and it fails for two independent reasons:1. nyc reports 0% coverage under ESM
nyc's instrumenter relied on the CommonJS-interop shims that #177 removed. With native ESM it captures nothing — 0% across all 14k+ statements.
2. The actual exit-1 cause: non-TTY progressBar crash
terminal-kitqueries the cursor location, which returnsInfinitywhen stdout is not an interactive TTY (CI, piped, redirected), then callsString.repeat(Infinity). Since.nycrc.jsonhadcheck-coverage: false, this failing test — not the 0% coverage — is what actually exited the step with code 1.Fix
.nycrc.json→.c8rc.jsonwith the same include/exclude/thresholds; adds thejsonreporter so codecov still receivescoverage/coverage-final.json. DropsnycdevDependency, addsc8.loadRealTerminalSync()inutils/base.jsto fall back to the console stub whenstdoutis not a TTY. This fixes the CI crash and real piped CLI usage (e.g.hana-cli ... > out.log).Verification (local)
npm run coverage:ciruns clean — exit 0.NODE_ENV=productionnon-TTY (piped) — throws before the fix, uses the stub after.coverage/coverage-final.jsonis produced for the codecov upload step.Notes
.github/instructions/*and rendereddocs/files still mentionnyc; those are documentation and don't affect CI. Can update in a docs follow-up if desired.