Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The implementation addresses the false-success paths with focused cross-platform regression coverage.
0 open findings
What changed in this PR
Ensures TypeScript documentation validation reliably fails for compiler startup, global, and external-file errors across platforms.
Changes:
- Invokes TypeScript through Node and normalizes diagnostic paths.
- Propagates compilation failures not tied to extracted examples.
- Adds regression tests and runs them in documentation CI.
| File | Description |
|---|---|
scripts/docs-validation/validate.ts |
Improves compiler execution and failure handling. |
scripts/docs-validation/validate.test.mjs |
Adds five real-compiler regression tests. |
scripts/docs-validation/package.json |
Adds the test command. |
.github/workflows/sdk-nodejs.yml |
Runs validator tests in Linux CI. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
This branch has not been deployed
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.
Problems
The TypeScript documentation validator catches compiler failures but only records diagnostics associated with extracted example files. A compiler startup failure, global diagnostic, or error confined to an imported file outside the examples can therefore result in every example being reported as valid and exit 0.
On Windows, directly executing
nodejs/node_modules/.bin/tscproduces ENOENT. The unchanged validator reported 201 files passed even with a deliberate TS2322 example. Bypassing only the launcher with Node also showed a real compiler exit 2 / TS2688 incorrectly reported as success.The Go validator writes an unquoted local replacement path into
go.mod. A checkout path containing spaces is split into separate tokens and rejected by the Go module parser.The C# validator has the same false-success behavior for build failures without an extracted
.csfile diagnostic. In an isolated fixture with a valid example and a malformed referenced project, realdotnet buildreturned MSB4025 / exit 1, but the unchanged validator reported all examples valid / exit 0. The C# code path was also checked against upstream341a526band is unchanged there.Changes
process.execPath, without a shell, and disable pretty diagnostics.go.mod.Validation
npm --prefix scripts/docs-validation test: 9 passed, 0 skipped.npm ci: the earlier TypeScript/Go suite passed 6 tests, 0 skipped before the C# additions.npm ci;node --test --test-name-pattern=C# validate.test.mjs: 3 passed, 0 skipped after the C# additions. Temporary tooling and fixtures were removed.go mod edit -jsonparsed the actual generatedgo.mod. Afterward it parses and the replacement path round-trips exactly.fixture.md:7location.git diff --checkpassed.Limits
The Go regression checks module generation and parsing, not compilation of the full Go documentation corpus. It skips locally if Go is absent; Go CI supplies Go.
The C# tests build small isolated projects targeting .NET 8 with package sources cleared. They require .NET 8 reference packs to be installed or cached and skip when
dotnetis absent. They do not establish that the complete C# documentation corpus compiles.The full
npm run docs:nodejscommand still encounters the existing local missing Node type definitions (TS2688). It now exits 1 and prints that diagnostic rather than falsely reporting all examples valid. I am not claiming the full documentation corpus compiles on this machine.