fix(dataset-tests): route DatasetTestRunner through the shared output emitter - #14
Merged
Conversation
… emitter The runner carried its own copy of the emitter. The copy had drifted: no title= or location prefix on the message, every non-failure level mapped to warning, and newlines flattened to spaces. The first of those is why a failing fixture named nothing in the job log even though its annotation anchored correctly. Routing through OutputEmitter corrects all three and, because Write already takes an optional FileAccounting, adds the coverage denominator the other runners report. Accounting is wired at the three exits the loop can take. Adds the project to Platform.slnx so a solution build compiles it and the suite can invoke it; it was previously built only by prepare-runner. Verdict is unchanged: the exit code and merged status are untouched.
They were written unconditionally to stdout, so a single missing fixture made --output json and --output sarif unparseable from their first character. The shared emitter already cites this diagnostic as the reason it guards its own coverage line; routing round it stopped being sufficient once the counts became part of the json payload. stderr rather than deletion, so the diagnostic still reaches the job log everywhere it was visible before. Found by the new coverage tests.
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.
DatasetTestRunnercarried its own copy of the shared output emitter, and the copy had drifted. It omittedtitle=and the location prefix on the message, which is the workaround the shared emitter documents for GitHub strippingfile=out of the rendered log line. So a failing dataset fixture produced an annotation that anchored correctly on the diff while the job log line named no file at all, and the log is what people actually read. It also mapped every non-failure level towarning, so a notice was reported as a warning, and it flattened newlines to spaces rather than%0A, so a nested result hierarchy arrived as one long line.Routing through
OutputEmitter.Writefixes all three.Writealready takes an optionalFileAccounting, so the same call site also gives this runner the coverage denominator the other runners report. There is no relevance filter in this runner, soNotRelevantstays zero by construction.The second commit stands alone deliberately. The two
[SKIP]diagnostics were written unconditionally to stdout, which corrupts--output jsonand--output sarif, since those put a single payload on stdout and nothing may precede it: one missing fixture made the output unparseable from its first character. This is pre-existing, and the shared emitter already cites this diagnostic as the reason it guards its own coverage line. Routing around it stopped being sufficient once the counts became part of the json payload. They now go to stderr, so the diagnostic still reaches the job log everywhere it was visible before. This affects--output jsonand--output sarifconsumers only; production passes--output githuband is unaffected either way.Verified on a Windows runner against a repository carrying dataset fixtures, with one fixture's expected output deliberately made wrong so the runner had a finding to emit, and with annotations read back from the check-runs API rather than from the log. The annotation gains a title carrying its location, the rendered log line gains the path prefix, and a coverage line appears; path, start line and annotation level are unchanged. The verdict is unchanged, checked on the mutated fixture and again across every fixture in that repository, with identical failure sets before and after.
The project is added to
Platform.slnx, which is what makes the tests possible: it was previously compiled only byprepare-runnerat check time, so no solution build covered it and nothing could invoke it. The emitted annotation shape is already covered by the existing emitter tests, so the new tests do not restate it; they cover that this runner routes through the shared emitter at all, and that accounting is wired at each exit the loop can take. The suite is green at 64 tests against 59 before.Not verified: the notice-to-warning mapping is a source reading. No notice-level finding arose in any run, so I have not observed that path.