Phase 2: PS7-only modernization & CI - #2
Merged
Conversation
- PSGraph.psd1: pin PowerShellVersion to '7.0'.
- Migrate Tests/ off legacy Pester 3/4 `Should X` syntax to Pester 5
`Should -X`, including several lowercase-verb and typo'd forms the
simple rename missed.
- Fix a real Pester 5 migration bug, not just syntax: several test files
computed shared fixture data (`$dot = graph g {...}`, `$functions =
Get-Command ...`) directly in Describe/Context bodies. Under Pester 5's
discovery/run split that data is discovery-only and evaluates to $null
when the It blocks actually run, so these tests were never actually
exercising anything. Fixed via BeforeAll (Export/Show-PSGraph.Tests.ps1)
and Pester 5's -ForEach data binding (Project.Tests.ps1, and
Tests/Project/{Help,Module}.Tests.ps1, the latter simplified from
one-It-per-ScriptAnalyzer-violation to one aggregate It per script).
- That fix surfaced two real, previously-never-run ScriptAnalyzer
findings: Entity.ps1 clobbered the automatic $matches variable and
lacked a process block (suppressed with justification - it converts
one InputObject into one Record by design, not a batch cmdlet), and
Node.ps1 trips PSAvoidOverwritingBuiltInCmdlets (suppressed like the
file's existing suppression, since renaming the DSL keyword is out of
scope here).
- Pin Pester to 5.7.1 in build.ps1 (this machine had 4.10.1 and 5.7.1
side by side, leaving Invoke-Pester resolution ambiguous) and add
-AllowClobber to the module installs, needed on any machine/runner
that already has a module providing the same command names as
BuildHelpers.
- Modernize BuildTasks/Pester.Task.ps1 from Pester 5's deprecated legacy
simple-parameter Invoke-Pester call to New-PesterConfiguration.
- Add .github/workflows/ci.yml: ubuntu/windows/macos x pwsh matrix,
installs Graphviz on every runner and runs the full Tests/ suite
untagged (not the -Tag Build filter BuildTasks/Pester.Task.ps1 uses
for the fast local dev loop) - closing a real gap where the
graphviz-tagged Export-PSGraph/Show-PSGraph tests, plus the untagged
Edge/Row tests, never actually ran under the old Tag-Build-filtered
pipeline.
- Remove psake.ps1 (confirmed dead: absent from build.ps1's module list
and unreferenced by both CI configs it's meant to serve) and the
appveyor.yml/azure-pipelines.yml it's superseded by GitHub Actions.
Verified: `Invoke-Pester -Path Tests` (all tags, Graphviz installed)
and `./build.ps1 Test` (Tag Build-filtered) both pass clean:
268/268 and 237/237 respectively.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014u9G91TefcQ42MAGvWrrqK
…macOS Tests built paths like "\$testdrive\bom.dot" with a literal backslash, which isn't a path separator on Linux/macOS. These graphviz-tagged tests never ran anywhere before Phase 2's CI actually installed Graphviz and exercised them cross-platform, so nothing caught it until now. Switched to Join-Path throughout. Confirmed via the actual GitHub Actions run on PR #2: this was the exact failure on ubuntu-latest and macos-latest (windows-latest already passed, since \ is a real separator there). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014u9G91TefcQ42MAGvWrrqK
4 tasks
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
1 participant
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.
Summary
PowerShellVersion = '7.0'inPSGraph.psd1.Tests/off legacy Pester 3/4Should Xsyntax to Pester 5Should -X(including several lowercase-verb / typo'd forms a naive rename would've missed).Describe/Contextbodies ($dot = graph g {...},$functions = Get-Command ...). Under Pester 5's discovery/run split that data is discovery-only and is$nullby the time theItblocks actually execute — so these tests were never really exercising anything, on any CI that ever ran them. Fixed withBeforeAll(Export-PSGraph.Tests.ps1,Show-PSGraph.Tests.ps1) and Pester 5's-ForEachdata binding (Tests/Project.Tests.ps1,Tests/Project/Help.Tests.ps1,Tests/Project/Module.Tests.ps1— the last simplified from one-It-per-ScriptAnalyzer-violation to one aggregateItper script).Entity.ps1clobbered the automatic$matchesvariable and lacks a process block (suppressed with a justification comment — it converts oneInputObjectinto oneRecordby design, not a batch cmdlet), andNode.ps1tripsPSAvoidOverwritingBuiltInCmdlets(suppressed like the file's existing suppression — renaming the DSL keyword is out of scope here).5.7.1inbuild.ps1and add-AllowClobberto the module installs (this dev machine had Pester 4.10.1 and 5.7.1 side by side, and a pre-existingMetadatamodule clash blockedBuildHelpersfrom installing without it).BuildTasks/Pester.Task.ps1off Pester 5's deprecated legacy simple-parameterInvoke-Pestercall toNew-PesterConfiguration..github/workflows/ci.yml: ubuntu/windows/macos ×pwshmatrix. Installs Graphviz on every runner and runs the entireTests/suite untagged (not the-Tag BuildfilterPester.Task.ps1uses for the fast local dev loop) — this closes a real gap where thegraphviz-taggedExport-PSGraph/Show-PSGraphtests, plus the untaggedEdge/Rowtests, never actually ran under the old-Tag Build-filtered pipeline.psake.ps1(confirmed dead: absent frombuild.ps1's module list and unreferenced by either CI config) andappveyor.yml/azure-pipelines.yml, superseded by GitHub Actions.Test plan
Invoke-Pester -Path Tests(all tags, Graphviz installed locally): 268/268 passed../build.ps1 Test(-Tag Build-filtered dev loop): 237 passed, 0 failed, 31 correctly skipped (graphviz-tagged).This is Phase 2 of
plans/modernization-roadmap.md, following Phase 1 (#1). Phase 3 (Graphviz coverage & PowerShell integration) is planned to follow once this merges.https://claude.ai/code/session_014u9G91TefcQ42MAGvWrrqK