From 44e1e3003d40451225457c8a08ca5e3634d61da7 Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Wed, 9 Sep 2026 10:29:44 +0100 Subject: [PATCH 1/3] fix(ci-compliance): exclude Versioning_ upgrade maps from dataset pathspec Follow-up to #20. That exclusion was path-based and covered one directory layout. Versioning_.json are versioning upgrade maps holding {"Dataset":{"ToNew":..,"ToOld":..}}, read by BHoM_Engine Versioning_Engine/Query/DatasetToNewPaths.cs. They carry no _t, can never be BH.oM.Data.Library.Dataset documents, and sit at a project root in any repo, so a path-scoped rule cannot reach them. Add ':(exclude,glob,icase)**/Versioning_[0-9]*.json' to both tier-bundle templates. Each element is required and is pinned by an assertion: :(glob) keeps the token a filename match rather than a path-substring match, the ** anchor keeps it depth-agnostic, and the [0-9] gate keeps a genuine dataset named Versioning_Rules.json in scope. Flip the two characterisation assertions #20 left as assert_matches, which its own comment recorded as the intended outcome of a widening. Add five fixtures covering the depth-2 case, a real dataset inside a Versioning_ directory, a real dataset under a Versioning_Engine project, and one path that the base selector never reaches. Refine the structural invariant to permit ** only under :(glob). Defect 3 is a property of git's default matcher, where '**' is not special; :(glob) switches on WM_PATHNAME and there '**/' matches zero or more leading components. The invariant previously rejected the only correct way to write a basename match. It still fails ** without :(glob). Suite 83 to 92 assertions. Fleet measurement over 309 repositories on their default branches: no repository loses a file, because no upgrade map currently sits in a selected path on a default branch. Refs BHoM/internal-tickets#36, BHoM/internal-tickets#43. --- .../tests/test-changed-file-patterns.sh | 97 ++++++++++++++++--- templates/BHE/ci-beta.yml | 18 ++-- templates/BHoM/ci-beta.yml | 18 ++-- 3 files changed, 94 insertions(+), 39 deletions(-) diff --git a/.github/scripts/tests/test-changed-file-patterns.sh b/.github/scripts/tests/test-changed-file-patterns.sh index 8fbbacb..ab970fd 100644 --- a/.github/scripts/tests/test-changed-file-patterns.sh +++ b/.github/scripts/tests/test-changed-file-patterns.sh @@ -10,8 +10,12 @@ # 2. A token with no wildcard is matched as a path prefix from the repo root, # NOT by basename. A bare 'AssemblyInfo.cs' therefore matches only a # repo-root file and misses every Properties/AssemblyInfo.cs. -# 3. '**' is not special to git. '**/' degrades to '*' plus a mandatory '/', -# so it requires at least one leading directory and skips root-level dirs. +# 3. '**' is not special to git BY DEFAULT. '**/' degrades to '*' plus a +# mandatory '/', so it requires at least one leading directory and skips +# root-level dirs. Under ':(glob)' it is special and '**/' matches zero or +# more leading components, which is the only correct way to write a +# basename match. The structural invariant at the foot of this file keys +# off that distinction rather than banning '**' outright. # # Both 2 and 3 shipped as live defects and silently under-selected files, which # means the affected checks passed without examining anything. Patterns are read @@ -66,10 +70,38 @@ FIXTURE_PATHS=( # for. These are BHoM versioning upgrade maps, {"Dataset":{"ToNew":..,"ToOld":..}}, # read by BHoM_Engine Versioning_Engine/Query/DatasetToNewPaths.cs from # %ProgramData%\BHoM\Upgrades. They carry no _t and cannot be Dataset documents. - # The first is FAILING IN PRODUCTION TODAY and the exclusion below deliberately - # does NOT cover it. See BHoM/internal-tickets#43. + # The first was the live production failure that motivated the Versioning_ + # exclusion. See BHoM/internal-tickets#43. "BuroHappold_Datasets/Versioning_93.json" "BHoM_Datasets/Versioning_93.json" + # The same map one level deeper, for a repo whose projects sit under src/. No + # fleet instance today: all 15 measured upgrade maps are at depth 1. Defensive, + # and it is what pins the '**' anchor: a single '*/' under :(glob) matches + # exactly one leading component and would pass every other assertion here. + "src/BuroHappold_Datasets/Versioning_93.json" + # POSITIVE, must stay selected. A real dataset inside a DIRECTORY named like an + # upgrade map. This is what :(glob) buys: without it the token's trailing '*' + # crosses '/', so this whole directory would leave scope. Measured 2026-09-09. + # Note there is deliberately no depth-zero fixture: a bare + # Versioning_.json at the repo root carries no 'datasets' substring, so + # the base selector never reaches it and any assertion on it would pass + # vacuously. + "Datasets/Versioning_93/RealDataset.json" + # NEGATIVE for the base selector, not for the exclusion. This is the layout a + # colleague cited as evidence the collision was fleet-wide: an upgrade map at a + # project root in a repo with no 'datasets' anywhere in the path. Measured + # 2026-09-09 over 309 repos: 15 such files in 11 repos, and NONE of them is + # selected, because the base selector needs the substring. Asserted so the + # example is recorded as inert rather than refiled as a bug. + "Physical_oM/Versioning_93.json" + # POSITIVE, must stay selected. The [0-9] gate is what keeps these in scope; a + # bare Versioning_* filename match would drop both. + # - a genuine dataset whose name happens to start with the same prefix + # - BHoM_Engine's dataset-test layout is .ci/Datasets/_Engine//, + # and Versioning_Engine is a real project there (18 source files, no dataset + # folder yet), so this path is one commit away from existing. + "Datasets/Versioning_Rules.json" + ".ci/Datasets/Versioning_Engine/Query/Thing.json" # Project-compliance inputs. "AssemblyInfo.cs" # repo root: the only thing the old token matched "Properties/AssemblyInfo.cs" # the real layout, was missed @@ -278,16 +310,30 @@ else # what keeps a Versioning_Toolkit dataset PR from failing on an unrelated cause. assert_not_matches "dataset" "$pat" ".ci/code/Versioning_Test/Datasets/9.2/Objects.json" assert_not_matches "dataset" "$pat" ".ci/code/Versioning_Test/Datasets/9.1/Methods.json" - # CHARACTERISATION, NOT APPROVAL. The project-directory case is still selected, - # and the first of these is a live production failure. Asserted positively for - # two reasons: it proves the exclusion above is one directory layout rather than - # a class-wide "stop checking non-datasets", and it means anyone who later widens - # the pattern into this case, or re-anchors the selector onto the data directory, - # has to flip a visible assertion instead of changing fleet-wide scope silently. - # If you are here because you re-anchored the selector: flipping these two to - # assert_not_matches is the intended outcome, not a regression. - assert_matches "dataset" "$pat" "BuroHappold_Datasets/Versioning_93.json" - assert_matches "dataset" "$pat" "BHoM_Datasets/Versioning_93.json" + # Versioning upgrade maps are excluded by filename, in any repo, at any depth. + # These two were assert_matches until 2026-09-09, as characterisation of a live + # production failure the earlier exclusion deliberately did not cover; the + # Versioning_ token covers it now, so they are flipped. + assert_not_matches "dataset" "$pat" "BuroHappold_Datasets/Versioning_93.json" + assert_not_matches "dataset" "$pat" "BHoM_Datasets/Versioning_93.json" + # Depth-agnostic: '**/' under :(glob) matches zero or more leading components. + # A single '*/' would match only depth 1 and pass every assertion above. + assert_not_matches "dataset" "$pat" "src/BuroHappold_Datasets/Versioning_93.json" + # The exclusion is a FILENAME match, and these three are what keeps it one. + # The first two are lost to ':(exclude,icase)*Versioning_*.json', which reads + # as a filename rule but is not one: without :(glob) the leading '*' crosses + # '/', so it drops every .json under any directory containing 'Versioning_'. + # The third is lost to the same token without :(glob) even with the [0-9] gate, + # because the TRAILING '*' crosses '/' and takes the directory's contents with + # it. All three measured 2026-09-09. + assert_matches "dataset" "$pat" "Datasets/Versioning_Rules.json" + assert_matches "dataset" "$pat" ".ci/Datasets/Versioning_Engine/Query/Thing.json" + assert_matches "dataset" "$pat" "Datasets/Versioning_93/RealDataset.json" + # Never selected in the first place: no 'datasets' substring anywhere in the + # path. Holds before and after the exclusion, and is asserted for exactly that + # reason. Do not read it as evidence the exclusion works; the three + # assert_not_matches above are that evidence. + assert_not_matches "dataset" "$pat" "Physical_oM/Versioning_93.json" done <<< "$dataset_patterns" fi @@ -326,6 +372,13 @@ else assert_not_matches "dataset-tests" "$dt_pattern" ".ci/code/Versioning_Test/Datasets/9.2/Objects.json" assert_not_matches "dataset-tests" "$dt_pattern" "BuroHappold_Datasets/Versioning_93.json" assert_not_matches "dataset-tests" "$dt_pattern" "BHoM_Datasets/Versioning_93.json" + assert_not_matches "dataset-tests" "$dt_pattern" "Physical_oM/Versioning_93.json" + assert_not_matches "dataset-tests" "$dt_pattern" "Datasets/Versioning_93/RealDataset.json" + # This one IS a fixture: it sits under the canonical directory and is a real + # Dataset document. It must stay in scope here while the compliance pattern also + # keeps it, so the two patterns cannot end up disagreeing about a real dataset + # merely because its project is called Versioning_Engine. + assert_matches "dataset-tests" "$dt_pattern" ".ci/Datasets/Versioning_Engine/Query/Thing.json" # The two patterns must stay distinct. If someone re-unifies them this fails. if [ "$dt_pattern" = "$(printf '%s' "$dataset_patterns" | head -1)" ]; then @@ -408,14 +461,28 @@ done # A wildcard-free token silently means "repo root only"; '**/' silently means # "at least one leading directory". Neither is ever intended for a basename # match. altConfigs.txt is the sole legitimate root-anchored file. +# +# The '**' rule is conditional on the token's magic, which is the one exception. +# Defect 3 is a property of git's DEFAULT matcher (wildmatch without +# WM_PATHNAME), where '**' is not special and '**/' degrades to '*' plus a +# mandatory '/'. ':(glob)' switches on WM_PATHNAME, and there '**/' is special +# and matches ZERO or more leading components. Measured 2026-09-09 on a fixture +# holding Versioning_93.json, Datasets/Versioning_93.json and +# a/b/c/Datasets/Versioning_93.json: +# ':(glob,icase)**/Versioning_[0-9]*.json' selects all three +# ':(icase)**/Versioning_[0-9]*.json' selects two, missing the root file +# So '**' under :(glob) is the only correct way to write a basename match here, +# and forbidding it outright would forbid the fix as well as the defect. echo "structural invariant over every shipped pattern token" ROOT_ANCHORED_OK="altConfigs.txt" while IFS= read -r pat; do [ -z "$pat" ] && continue for tok in $pat; do case "$tok" in + :\(*glob*\)*'**'*) + pass "invariant: '$tok' uses '**' under :(glob), where it matches zero or more leading components" ;; *'**'*) - fail "invariant: '$tok' uses '**', which git reads as '*' plus a mandatory '/' (skips root-level dirs)" ;; + fail "invariant: '$tok' uses '**' without :(glob), which git reads as '*' plus a mandatory '/' (skips root-level dirs)" ;; *'*'*|*'?'*|*'['*|:\(*\)*) pass "invariant: '$tok' is wildcarded or uses pathspec magic" ;; "$ROOT_ANCHORED_OK") diff --git a/templates/BHE/ci-beta.yml b/templates/BHE/ci-beta.yml index 91c401c..26b329f 100644 --- a/templates/BHE/ci-beta.yml +++ b/templates/BHE/ci-beta.yml @@ -54,18 +54,12 @@ jobs: uses: BHoM/CI_Toolkit/.github/actions/ci-compliance@develop with: check_type: dataset - # Versioning_Test/Datasets holds JSON Lines of oM objects, not - # BH.oM.Data.Library.Dataset documents, so IsValidDataset errors on every - # one of them. Measured: DatasetComplianceRunner exits 1 on 9.2/Objects.json - # and 0 on a real library dataset. Excluded rather than teaching the runner - # a second shape, because that would touch Test_Toolkit. See - # BHoM/internal-tickets#36. - # - # This excludes one directory layout, not a class of file. The selector is - # substring-based, not dataset-aware: '*' crosses '/' without :(glob), so - # the substring can come from a filename, a data directory, or a project - # directory named *_Datasets. See BHoM/internal-tickets#43. - patterns: ':(icase)*datasets*.json :(exclude,icase)*Versioning_Test/Datasets/*' + # Excludes two shapes that match this selector but are not + # BH.oM.Data.Library.Dataset documents: Versioning_Test/Datasets, which is + # JSON Lines of oM objects, and Versioning_.json upgrade maps. + # Keep both controls on the last token: :(glob) stops its wildcards + # crossing '/', and [0-9] keeps a real Versioning_Rules.json in scope. + patterns: ':(icase)*datasets*.json :(exclude,icase)*Versioning_Test/Datasets/* :(exclude,glob,icase)**/Versioning_[0-9]*.json' app_id: ${{ secrets.BHOM_APP_ID }} private_key: ${{ secrets.BHOM_APP_PRIVATE_KEY }} diff --git a/templates/BHoM/ci-beta.yml b/templates/BHoM/ci-beta.yml index 21d53af..e825821 100644 --- a/templates/BHoM/ci-beta.yml +++ b/templates/BHoM/ci-beta.yml @@ -70,18 +70,12 @@ jobs: uses: BHoM/CI_Toolkit/.github/actions/ci-compliance@develop with: check_type: dataset - # Versioning_Test/Datasets holds JSON Lines of oM objects, not - # BH.oM.Data.Library.Dataset documents, so IsValidDataset errors on every - # one of them. Measured: DatasetComplianceRunner exits 1 on 9.2/Objects.json - # and 0 on a real library dataset. Excluded rather than teaching the runner - # a second shape, because that would touch Test_Toolkit. See - # BHoM/internal-tickets#36. - # - # This excludes one directory layout, not a class of file. The selector is - # substring-based, not dataset-aware: '*' crosses '/' without :(glob), so - # the substring can come from a filename, a data directory, or a project - # directory named *_Datasets. See BHoM/internal-tickets#43. - patterns: ':(icase)*datasets*.json :(exclude,icase)*Versioning_Test/Datasets/*' + # Excludes two shapes that match this selector but are not + # BH.oM.Data.Library.Dataset documents: Versioning_Test/Datasets, which is + # JSON Lines of oM objects, and Versioning_.json upgrade maps. + # Keep both controls on the last token: :(glob) stops its wildcards + # crossing '/', and [0-9] keeps a real Versioning_Rules.json in scope. + patterns: ':(icase)*datasets*.json :(exclude,icase)*Versioning_Test/Datasets/* :(exclude,glob,icase)**/Versioning_[0-9]*.json' app_id: ${{ secrets.BHOM_APP_ID }} private_key: ${{ secrets.BHOM_APP_PRIVATE_KEY }} From 4e61fefa8c28978f34501a86707413c7d9d33deb Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Mon, 28 Sep 2026 17:58:02 +0100 Subject: [PATCH 2/3] test(ci-compliance): use a neutral name for the *_Datasets fixture paths The pathspec fixtures named a private repository. This is a public repository, so the name is replaced with Example_Datasets. BHoM_Datasets stays as the real, public anchor for the same case. The fixtures assert a path shape: a directory component supplying the 'datasets' substring plus a Versioning_.json filename. The repository name is arbitrary, so nothing the suite asserts changes. Verified by running the suite before and after the substitution: 92 of 92 both times. Two comments tied a fixture to one specific live failure. They now state what makes the fixture the shape it is instead. --- .../tests/test-changed-file-patterns.sh | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/scripts/tests/test-changed-file-patterns.sh b/.github/scripts/tests/test-changed-file-patterns.sh index ab970fd..0cdf773 100644 --- a/.github/scripts/tests/test-changed-file-patterns.sh +++ b/.github/scripts/tests/test-changed-file-patterns.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# test-changed-file-patterns.sh — asserts the changed-file patterns this repo +# test-changed-file-patterns.sh: asserts the changed-file patterns this repo # ships behave correctly when handed to `git diff -- `. # # compute-changed-files does `read -ra pathspecs <<< "$PATTERNS"` and passes the @@ -51,7 +51,7 @@ FIXTURE_PATHS=( "src/datasets/lower/low.json" # lowercase, already matched "a/mydatasets.json" # substring in filename, already matched "Datasets/root-direct.json" # root-level, e.g. CFD_Toolkit - "Datasets/Area/root-nested.json" # root-level nested, e.g. BuroHappold_Datasets + "Datasets/Area/root-nested.json" # root-level nested, e.g. a *_Datasets repo "DataSets/Caps/caps.json" # root + capital S, e.g. LifeCycleAssessment_Toolkit "DATASETS/shouty/shout.json" # upper case, defensive "datasets.json" # root-level file @@ -70,15 +70,15 @@ FIXTURE_PATHS=( # for. These are BHoM versioning upgrade maps, {"Dataset":{"ToNew":..,"ToOld":..}}, # read by BHoM_Engine Versioning_Engine/Query/DatasetToNewPaths.cs from # %ProgramData%\BHoM\Upgrades. They carry no _t and cannot be Dataset documents. - # The first was the live production failure that motivated the Versioning_ - # exclusion. See BHoM/internal-tickets#43. - "BuroHappold_Datasets/Versioning_93.json" + # Both sit in a project folder named *_Datasets, which is what supplies the + # 'datasets' substring the base selector needs. + "Example_Datasets/Versioning_93.json" "BHoM_Datasets/Versioning_93.json" # The same map one level deeper, for a repo whose projects sit under src/. No # fleet instance today: all 15 measured upgrade maps are at depth 1. Defensive, # and it is what pins the '**' anchor: a single '*/' under :(glob) matches # exactly one leading component and would pass every other assertion here. - "src/BuroHappold_Datasets/Versioning_93.json" + "src/Example_Datasets/Versioning_93.json" # POSITIVE, must stay selected. A real dataset inside a DIRECTORY named like an # upgrade map. This is what :(glob) buys: without it the token's trailing '*' # crosses '/', so this whole directory would leave scope. Measured 2026-09-09. @@ -311,14 +311,14 @@ else assert_not_matches "dataset" "$pat" ".ci/code/Versioning_Test/Datasets/9.2/Objects.json" assert_not_matches "dataset" "$pat" ".ci/code/Versioning_Test/Datasets/9.1/Methods.json" # Versioning upgrade maps are excluded by filename, in any repo, at any depth. - # These two were assert_matches until 2026-09-09, as characterisation of a live - # production failure the earlier exclusion deliberately did not cover; the - # Versioning_ token covers it now, so they are flipped. - assert_not_matches "dataset" "$pat" "BuroHappold_Datasets/Versioning_93.json" + # These two were assert_matches until 2026-09-09, characterising a case the + # earlier exclusion deliberately did not cover; the Versioning_ token + # covers it now, so they are flipped. + assert_not_matches "dataset" "$pat" "Example_Datasets/Versioning_93.json" assert_not_matches "dataset" "$pat" "BHoM_Datasets/Versioning_93.json" # Depth-agnostic: '**/' under :(glob) matches zero or more leading components. # A single '*/' would match only depth 1 and pass every assertion above. - assert_not_matches "dataset" "$pat" "src/BuroHappold_Datasets/Versioning_93.json" + assert_not_matches "dataset" "$pat" "src/Example_Datasets/Versioning_93.json" # The exclusion is a FILENAME match, and these three are what keeps it one. # The first two are lost to ':(exclude,icase)*Versioning_*.json', which reads # as a filename rule but is not one: without :(glob) the leading '*' crosses @@ -370,7 +370,7 @@ else # project-directory case was ever in scope here. Asserted so that re-anchoring # this pattern cannot silently pull them in. assert_not_matches "dataset-tests" "$dt_pattern" ".ci/code/Versioning_Test/Datasets/9.2/Objects.json" - assert_not_matches "dataset-tests" "$dt_pattern" "BuroHappold_Datasets/Versioning_93.json" + assert_not_matches "dataset-tests" "$dt_pattern" "Example_Datasets/Versioning_93.json" assert_not_matches "dataset-tests" "$dt_pattern" "BHoM_Datasets/Versioning_93.json" assert_not_matches "dataset-tests" "$dt_pattern" "Physical_oM/Versioning_93.json" assert_not_matches "dataset-tests" "$dt_pattern" "Datasets/Versioning_93/RealDataset.json" From 46341b8b6fbdfea7cdf3dfda2eb03705dbcb1cd8 Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Mon, 28 Sep 2026 18:09:49 +0100 Subject: [PATCH 3/3] test(ci-compliance): drop an unreachable ticket reference from the fixtures This is a public repository and the reference pointed into a private one, so no outside reader could open it. It was also mis-aimed: the ticket it named covers a missing Datasets.txt and a fail-open path, not the decision this comment explains. Nothing is lost. The two lines above the reference already state the reason the shape is excluded rather than taught to the runner. Suite unchanged at 92 of 92. --- .github/scripts/tests/test-changed-file-patterns.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/scripts/tests/test-changed-file-patterns.sh b/.github/scripts/tests/test-changed-file-patterns.sh index 0cdf773..e12d167 100644 --- a/.github/scripts/tests/test-changed-file-patterns.sh +++ b/.github/scripts/tests/test-changed-file-patterns.sh @@ -61,7 +61,6 @@ FIXTURE_PATHS=( # BH.oM.Data.Library.Dataset documents, so IsValidDataset errors on every one. # Measured 2026-09-07: DatasetComplianceRunner exits 1 on this file and 0 on a # real library dataset. Excluded rather than teaching the runner a second shape. - # See BHoM/internal-tickets#36. ".ci/code/Versioning_Test/Datasets/9.2/Objects.json" ".ci/code/Versioning_Test/Datasets/9.1/Methods.json" # The project-directory case. Here the substring comes from neither the filename