Share explicit build test project list

[angularsen]

Motivation

The build scripts should keep the actual test project list easy to see and avoid project discovery or MSBuild evaluation on every build. The net48 compatibility workflow runs the same main test projects on .NET Framework, so it should not duplicate the project names or import the full build-functions module just to get the list.

Changes

• move the hard-coded main test project list to Build/test-projects.psm1
• expose the list through Get-TestProjectPaths
• call Get-TestProjectPaths explicitly from Build/build-functions.psm1 and the net48 compatibility workflow
• merge the branch onto latest master

Validation

• powershell -NoProfile -ExecutionPolicy Bypass -Command 'Import-Module ./Build/test-projects.psm1 -Force; @(Get-TestProjectPaths) | ForEach-Object { $_ }'
• pwsh -NoProfile -Command 'Import-Module ./Build/test-projects.psm1 -Force; @(Get-TestProjectPaths) | ForEach-Object { $_ }'
• powershell -NoProfile -ExecutionPolicy Bypass -Command 'Import-Module ./Build/build-functions.psm1 -Force'
• pwsh -NoProfile -Command 'Import-Module ./Build/build-functions.psm1 -Force'
• git diff --check -- Build/test-projects.psm1 Build/build-functions.psm1 .github/workflows/net48-compatibility.yml

[claude]

Review

This is a build-tooling-only change (Build/build-functions.psm1, .github/workflows/net48-compatibility.yml) — no quantity/unit JSON, generated code, or public API changes, so there's nothing to check against the units/quantities criteria and no breaking changes.

What it does well

• Replaces two independently-maintained hardcoded project lists (the exact drift bug called out in the motivation, from Allow configuring global default quantities #1692) with a single Get-TestProjects discovery function shared by both the primary and net48 pipelines.
• Adds if ($testProjects.Count -eq 0) { throw ... } guards that didn't exist before — a good defensive addition so a discovery regression fails loudly instead of silently skipping all tests.
• Sort-Object FullName keeps discovery output deterministic across OS/filesystem enumeration order.
• Correctly accounts for the Condition="$([MSBuild]::IsOSPlatform('Windows'))" net48 addition in the test csproj files — net48 discovery only works on the Windows runner, which matches where net48-compatibility.yml actually runs (runs-on: windows-latest).

Minor nits (non-blocking)

• Build/build-functions.psm1:50: @($properties.Properties.TargetFrameworks -split ';') + $properties.Properties.TargetFramework doesn't trim whitespace after splitting. If a future <TargetFrameworks>net10.0; net48</TargetFrameworks> (space after ;) is ever added, -contains 'net48' would silently fail to match due to the leading space. Consider ... | ForEach-Object { $_.Trim() } to make this robust against that formatting variant.
• Same line: when a project only declares TargetFrameworks (no singular TargetFramework, the common case here), $properties.Properties.TargetFramework is empty/$null and still gets appended to the array. Harmless today since it never matches -Framework, but + @($properties.Properties.TargetFramework) | Where-Object { $_ } would keep the array purely meaningful values.
• Get-ChildItem -Path $root -Recurse -Filter "*.Tests.csproj" walks the entire repo tree (including .git, Artifacts, bin/obj) on every invocation. Not a correctness issue today with only 4 test projects, but worth keeping in mind if the repo grows — could scope to known source directories or add -Exclude/depth limits later.
• Each discovered project incurs its own dotnet msbuild -getProperty process spin-up. Fine at this scale (4 projects), but something to watch if the test-project count grows significantly.

Test coverage: Reasonable for a CI script change — author validated manually (discovery returns all 4 projects for both TFMs, PS 7/5.1, full net10 run, partial net48 run) per the PR description. No automated test for Get-TestProjects itself, but that's a normal tradeoff for build tooling and CI will exercise the real path on merge.

Overall: solid, well-scoped DRY fix for the exact problem it says it's solving. No blocking issues found.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98%. Comparing base (c88bc60) to head (0694798).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #1693     +/-   ##
========================================
+ Coverage      97%     98%     +1%     
========================================
  Files         237     514    +277     
  Lines       18503   24141   +5638     
========================================
+ Hits        18013   23748   +5735     
+ Misses        490     393     -97     

Flag	Coverage Δ
net48	97% <ø> (+<1%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[claude]

Review

This PR is scoped entirely to build tooling (Build/build-functions.psm1, .github/workflows/net48-compatibility.yml) — no unit/quantity JSON definitions or generated code are touched, so most of the usual quantity/generator review checklist doesn't apply here. No breaking changes to the public library API.

What it does well

• Assert-TestProjectsAreListed is a good, low-cost guard: I verified locally that git ls-files "*.Tests.csproj" returns exactly the 10 projects enumerated in $knownTestProjectPaths (7 main + 3 UnitsNet.Modular), so the guard passes cleanly against the current tree and would correctly fail if a new *.Tests.csproj were added without updating the list.
• ConvertTo-RepoRelativePath correctly guards against partial-prefix false matches (e.g. a sibling dir named root2 matching a root of root) by comparing against root + DirectorySeparatorChar, and normalizes \// for cross-platform comparisons. Nice attention to detail.
• Exporting Assert-TestProjectsAreListed from the module and reusing it in both Start-Tests and the net48 workflow avoids duplicating the check logic.

Minor issues

1. Gap in the stated goal: the PR motivation says "adding a new *.Tests.csproj should not silently leave CI incomplete," but Assert-TestProjectsAreListed only checks that every *.Tests.csproj is listed somewhere in $knownTestProjectPaths. It does not check that the net48 workflow's own separate, hardcoded $testProjects list (net48-compatibility.yml:56-63) stays in sync with $mainTestProjectPaths. In fact UnitsNet.Serialization.SystemTextJson.Tests is already absent from that net48 list today — pre-existing (not introduced by this PR), but the new guard won't catch that kind of drift, so the "silently incomplete" problem is only partially solved. Worth a one-line comment in the workflow noting the exclusion is intentional (if it is), or extending the guard to cross-check this list too.
2. Get-ChildItem -Recurse over the whole repo root on every Start-Tests invocation (Build/build-functions.psm1:81) walks .git, Artifacts, and all bin/obj folders. It's fine today (I confirmed no false positives locally), but it's a bit more I/O than necessary — filtering out .git, Artifacts, bin, obj (or scanning only known project-root-level directories) would make it more robust against future noise (e.g. a NuGet package that ships a sample *.Tests.csproj into a restored obj folder) and slightly faster.
3. Style nit: $knownTestProjectPaths (build-functions.psm1:22-27) mixes array-literal styles — $mainTestProjectPaths is spliced in without a trailing comma while the following three entries use commas. This is valid PowerShell (newline-separated statements flatten into the outer array same as comma-separated), but it reads as inconsistent/easy to typo when someone next edits this list.

Test coverage

No automated test exercises Assert-TestProjectsAreListed or ConvertTo-RepoRelativePath directly (understandably, since there's no PowerShell test harness in this repo) — validation is manual per the PR description. Given this function is a correctness guard for CI itself, that seems like an acceptable trade-off for a script of this size.

Overall: a reasonable, self-contained improvement to CI robustness with no impact on the library itself. The two minor points above are worth a look but not blocking.

[claude]

Review

Small, low-risk build-script change — no impact on the public API, generated code, or unit/quantity definitions, so most of the usual review checklist (breaking changes, new units, generator changes) doesn't apply here.

What it does: hoists the Start-Tests project list out of the function body into a top-level $mainTestProjectPaths variable in Build/build-functions.psm1, and points $projectPaths at it.

Feedback

• Naming inconsistency: Start-PackNugets (line 110) still declares its own local $projectPaths list inline, right below the new top-level $mainTestProjectPaths. Having one list hoisted to module scope and another left local, both assigned to a same-named local variable $projectPaths, is a bit inconsistent — a reader skimming the module top might expect the pack-nuget list to be hoisted too, or wonder why it wasn't. Not a blocker (PR description only claims to change the test list), but worth a one-line comment or follow-up if the intent is to eventually consolidate.
• Redundant indirection: $projectPaths = $mainTestProjectPaths (line 56) is functionally a no-op rename — the function could just use $mainTestProjectPaths directly and drop the local variable, saving a line. Minor, and keeping $projectPaths does preserve the internal name used later in the function, so this is a matter of taste.
• Net48 workflow duplication: the PR intentionally leaves .github/workflows/net48-compatibility.yml's own hard-coded copy of this same list untouched (confirmed — it still has its own inline list at lines 54-59). Since this PR is explicitly about making the test list "easy to see" and avoiding duplication, it might be worth a follow-up issue to share the list between the PowerShell module and the YAML workflow (e.g., generate the YAML matrix from the same source, or vice versa), since today a new test project added to one list can silently be missed in the other. Not something this PR needs to solve, just flagging the residual duplication.

Non-issues checked

• No breaking changes (build-only script, not shipped).
• No generated-code or unit-definition changes.
• Style/formatting is consistent with the rest of the file (2-space indent, write-host -foreground blue banners, etc.).
• No security concerns.

Overall this is a reasonable, low-risk cleanup that does what the description says.

🤖 Generated with Claude Code

[claude]

Review

Small, well-scoped build-infra change — no C#, JSON unit definitions, or generated code touched, so most of the review checklist (breaking changes, quantities/units, generated-code diffs) doesn't apply here.

Breaking changes: None — this only affects build/CI scripts, not the published library.

Behavior change worth confirming intentional: The net48 workflow's hardcoded list previously had 6 projects and excluded UnitsNet.Serialization.SystemTextJson.Tests, while Start-Tests in build-functions.psm1 had 7 and included it. By consolidating onto the single Build/test-projects.ps1 list, net48-compatibility.yml now also builds/tests UnitsNet.Serialization.SystemTextJson.Tests under net48. That project does target net48 in its TargetFrameworks, so this looks like a net-positive coverage fix rather than a bug, but worth calling out explicitly in the PR description since it's a behavioral side effect of the consolidation, not just a refactor.

Style/conventions:

• Dot-sourcing via $PSScriptRoot in build-functions.psm1:12 is correct — $PSScriptRoot resolves to the module's own directory, so Join-Path $PSScriptRoot "test-projects.ps1" correctly points at Build/test-projects.ps1 regardless of caller's CWD.
• . ./Build/test-projects.ps1 in net48-compatibility.yml is relative to the job's working directory instead of $PSScriptRoot-based like the module does. Since the workflow always runs from the repo root (post-checkout), this works, but it's a slightly inconsistent sourcing style vs. the $PSScriptRoot-based approach in build-functions.psm1 — could drift if someone adds a working-directory: override to that step later. Not a blocker, just a minor consistency nit.
• New Build/test-projects.ps1 uses CRLF line endings and no BOM, matching .gitattributes (*.ps1 eol=crlf) and the sibling .ps1 scripts in Build/. build-functions.psm1 has a BOM but that's not enforced by .gitattributes, so no issue.
• Good catch consolidating the third variant UnitsNet.Serialization.SystemTextJson.Tests list wasn't duplicated anywhere else (ci.yml etc. run via dotnet test UnitsNet.slnx), so no leftover duplication remains after this change.

Test coverage: N/A (build tooling only) — the PR description lists manual validation commands for both Windows PowerShell and pwsh, which is appropriate given this can't easily be unit tested.

Overall this is a clean, minimal refactor that does what the description says. Nice job keeping the list explicit rather than switching to project discovery/MSBuild evaluation, which avoids extra build-time cost per the stated motivation.

[claude]

Review

Scope: this PR only touches build tooling (Build/build-functions.psm1, new Build/test-projects.ps1, .github/workflows/net48-compatibility.yml) — no changes to quantities, units, generated code, or public APIs, so most of the usual review checklist (breaking changes, new units, generated-code diffs) doesn't apply here.

Behavior change worth flagging

The net48 workflow's hard-coded list previously had 6 projects and was missing UnitsNet.Serialization.SystemTextJson.Tests, while Start-Tests in build-functions.psm1 already ran 7 (including it). By switching both to the shared Get-TestProjectPaths, the net48 workflow now also builds/tests UnitsNet.Serialization.SystemTextJson.Tests on net48.

This looks like a good fix for a pre-existing inconsistency (the csproj already multi-targets net48;net8.0;net9.0;net10.0, so it should build fine), but it's a scope change not mentioned in the PR description's bullet list, and it adds another dotnet build/dotCover pass to the net48 CI job (runtime + coverage upload). Worth calling out explicitly in the PR description so reviewers know it's intentional, and worth confirming the net48-compatibility CI run on this PR is green for that project specifically.

Style / consistency

• Get-TestProjectPaths follows PowerShell's approved-verb convention and the new file's 2-space indentation matches the rest of Build/.
• Dot-sourcing test-projects.ps1 and caching $testProjectPaths at module scope in build-functions.psm1 mirrors the existing pattern for $root/$artifactsDir/etc. in that file, so it's consistent with the surrounding code.
• Minor nit: other Build/*.ps1 entry points (clean.ps1, init.ps1) start with Set-Strictmode -version latest; the new test-projects.ps1 doesn't. Not a blocker since it's a tiny data-only file, just noting for consistency.

Correctness

Good catch on the underlying goal (single source of truth for the test project list) — this removes real duplication risk where the three lists could silently drift, which is exactly what happened here (net48 workflow silently missing a project).

Test coverage / validation

The PR description's validation steps confirm the function resolves correctly and the module imports cleanly, but don't call out that the net48 job will now run an extra test project. Recommend explicitly confirming the net48-compatibility CI result on this PR shows UnitsNet.Serialization.SystemTextJson.Tests passing under net48 with coverage collection.

No breaking changes, security concerns, or performance issues identified.

[claude]

Review

Small, well-scoped build-tooling refactor — no library code, generated code, or unit/quantity definitions touched.

Breaking changes: None. This only affects internal build/CI scripts, not the published packages or public API.

Behavior change worth calling out: The net48 workflow's old hard-coded list was missing UnitsNet.Serialization.SystemTextJson.Tests.csproj (present in build-functions.psm1's list but not in net48-compatibility.yml). After this change, both consume Get-TestProjectPaths, so the net48 workflow will now also build/test that project. I confirmed UnitsNet.Serialization.SystemTextJson.Tests.csproj already targets net48;net8.0;net9.0;net10.0, so this looks like a correct convergence rather than an accidental scope change — worth a one-line callout in the PR description so reviewers don't mistake it for unintentional, since it does add one more project to the net48 CI matrix (slightly longer CI time).

Style/conventions: Consistent with the existing PowerShell in Build/. Export-ModuleMember usage, Get-Verb-Noun naming, and quoting style all match build-functions.psm1.

Minor nit: build-functions.psm1 does Import-Module (Join-Path $PSScriptRoot "test-projects.psm1") without -Force. Not an issue for CI (fresh process each run), but if this module is ever dot-sourced repeatedly in a long-lived shell session (e.g. local interactive dev), a stale cached version could be picked up after edits. Could add -Force for consistency with how modules are typically re-imported in this repo, but low priority.

Generated code: N/A — no changes to CodeGen or GeneratedCode/.

New quantities/units: N/A.

Test coverage: N/A for a build-script change; the PR description documents manual validation (Import-Module ... -Force smoke tests plus git diff --check), which is reasonable given there's no automated test harness for these PowerShell modules.

Security: None — no untrusted input, no new external calls, purely internal script consolidation.

Overall this is a clean DRY-up that removes a real duplication bug (the net48/build-functions list drift). LGTM once the SystemTextJson.Tests inclusion is confirmed intentional.

[claude]

Review

This is a build-tooling-only change (PowerShell scripts + workflow YAML) — no impact on the public API, quantities/units, or generated code, so most of the usual review checklist doesn't apply here. Nice cleanup of the duplicated test-project list.

Behavior change worth confirming (not just a refactor)

Before this PR, net48-compatibility.yml and Build/build-functions.psm1 had different hard-coded lists:

• The net48 workflow ran 6 projects and did not include UnitsNet.Serialization.SystemTextJson.Tests.
• build-functions.psm1 (used by build.bat/local builds) ran 7 projects, including it.

After this PR, both consume the same shared Get-TestProjectPaths (7 projects), so UnitsNet.Serialization.SystemTextJson.Tests will now run on net48 in CI for the first time. That project does target net48 in its TargetFrameworks, so it should build/run fine, but this is a functional change to CI coverage (and CI time), not just a de-duplication of an identical list. Worth calling out explicitly in the PR description, and worth double-checking CI passes for that project on net48 (System.Text.Json behavior can differ subtly on .NET Framework vs. modern runtimes).

Minor style notes

• Build/test-projects.psm1 uses Export-ModuleMember (PascalCase) while Build/set-version.psm1 uses export-modulemember (lowercase) — inconsistent casing across the two modules in the same folder, though functionally irrelevant since PowerShell cmdlets are case-insensitive.
• In build-functions.psm1, $testProjectPaths is computed once at module-import time (line 13) rather than inside Start-Tests. Minor coupling to import order, but not a real issue given Import-Module -Force is called right above it.

Test coverage

No automated tests are applicable here (build scripts), and the PR description lists reasonable manual validation (importing both modules under powershell and pwsh). That seems sufficient for this kind of change.

No breaking changes, security concerns, or issues with the generated-code pipeline — this PR doesn't touch CodeGen or Common/UnitDefinitions/.