Skip to content

fix(dataset-tests): route DatasetTestRunner through the shared output emitter - #14

Merged
sakanni merged 2 commits into
developfrom
fix/dataset-runner-output
Aug 26, 2026
Merged

fix(dataset-tests): route DatasetTestRunner through the shared output emitter#14
sakanni merged 2 commits into
developfrom
fix/dataset-runner-output

Conversation

@sakanni

@sakanni sakanni commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

DatasetTestRunner carried its own copy of the shared output emitter, and the copy had drifted. It omitted title= and the location prefix on the message, which is the workaround the shared emitter documents for GitHub stripping file= 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 to warning, 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.Write fixes all three. Write already takes an optional FileAccounting, so the same call site also gives this runner the coverage denominator the other runners report. There is no relevance filter in this runner, so NotRelevant stays zero by construction.

The second commit stands alone deliberately. The two [SKIP] diagnostics were written unconditionally to stdout, which corrupts --output json and --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 json and --output sarif consumers only; production passes --output github and 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 by prepare-runner at 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.

… 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.
@sakanni
sakanni merged commit 064bd44 into develop Aug 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant