Repository navigation
Report Confidence as a warning for unit-test dataset fixtures - #40
Merged
Merged
Conversation
The Confidence rule errors on any dataset whose source information leaves the field Undefined. That is right for a dataset a repository distributes and wrong for a fixture that only drives its own tests, where blocking a merge on it costs more than it buys. A dataset under a .ci directory now reports the finding as a warning; everything else, including a dataset that does not deserialise, is unchanged. The demotion sits in the runner rather than in IsValidDataset because the engine has no concept of a unit-test fixture. It runs before the merge because Modify.Merge combines only the two top-level statuses, so demoting the annotation alone would leave the check red with nothing to account for it. Scoped to any .ci descendant rather than .ci/Datasets alone, because the repository convention puts test projects under .ci/unit-tests/ and a fixture beside one is test data by the same argument. Measured across 340 clones and 1694 selector-matching dataset files: the two spellings pick the same 865 files in the same 15 repositories, so this closes a class without changing an outcome. A run that demoted anything says so, for the same reason a self-skipping job names its pathspec: a green that examined nothing and a green that downgraded fifty-five findings should not look alike. 48 tests added. The demotion keys on a rule name owned by Test_Toolkit, which is the one fragility in the design, and it fails safe: a rename upstream stops the match, every fixture returns to Error and the check gets stricter rather than quieter. The pin test catches the rename on the next pull request here that touches tools/, which is when this suite runs, rather than when Test_Toolkit changes.
sakanni
marked this pull request as ready for review
October 25, 2026 21:09
This was referenced Oct 25, 2026
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.
Issues addressed by this PR
Closes #39
Test files
FileFilterTests,DatasetSeverityTestsandDatasetComplianceRunnerE2ETests, 48 new tests, no existing test modified. 131 passing inCompliance.Unit.Testsagainst a baseline of 108, and 99 inCompliance.Testsagainst 74, with nothing skipped.Checked against a mutation rather than assumed: disabling the demotion call fails exactly three of them, and the other dataset end-to-end tests still pass because the mutation does not change what they describe.
Changelog
Confidenceas a warning for unit-test fixtures under.ci, and still as an error for every other datasetAdditional comments
IsValidDataseterrors on any dataset whose source information leavesConfidenceatUndefined, which is right for a dataset a repository ships and wrong for a fixture driving its own tests. A dataset under a.cidirectory now reports that finding as a warning. Nothing else moves: a fixture that fails to deserialise still fails the check, the other source-information rules are untouched, and the field is still expected on both kinds.The demotion is in the runner rather than in
IsValidDataset, because the rule is right about the dataset either way and what changes is a judgement about what the file is for.Scope is any
.cidescendant, not.ci/Datasetsalone, because the repository convention also puts test projects under.ci/unit-tests/. Measured across 340 clones and 1,694 selector-matching dataset files, the two readings pick the same files today, so this closes a class rather than changing an outcome.The demotion keys on the rule name
IsValidDatasetemits, which is the one fragile part. It fails safe: a rename upstream stops the match, every fixture goes back to erroring, and the check gets stricter rather than quieter. There is a test pinning the name against the live engine, and it runs on the next pull request here that touchestools/, not whenTest_Toolkitchanges.A run that demotes anything now says so, on the same argument that put a summary line on a self-skipping job. Note it reaches the log and the run's annotations but not the job summary, which the action assembles from
Coverage:lines only.On a repository that has not fixed its fixtures this turns a red check green, which is the intent. Measured on the repositories we hold: 55 of 126 findings become warnings and one repository stops failing outright.