Fix integration suite for module rename; harden repo for public use - #35
Merged
Conversation
added 17 commits
August 10, 2026 11:26
Microsoft's Microsoft.Adapter/PowerShell runtime adapter and DscResource.Authoring's manifest generator both discover class-based DSC resources via reflection/AST inspection of each class file, which can't see Set()/Test() when those are implemented purely on a shared base class - verified empirically that every one of this module's 49 resources was only advertising "get" capability to DSC v3 tooling, even though Set/Test work fine when invoked directly. Adds a direct Set()/Test() override to each resource class (delegating to AzDevOpsDscResourceBase, replacing the PSScriptAnalyzer suppression that previously excused their absence) so both the adapter and the manifest generator correctly detect get/set/test. Verified the delegation is a functional no-op, including that virtual dispatch to each resource's overridden hook methods still resolves correctly when called from inside the base class's Set()/Test(). Adds DscResource.Authoring as a build dependency and a new `dscv3` build workflow that generates DSC v3 adapted resource manifests for all 49 resources into the build output, wired into `pack` so every packaged release ships them. Manifests are generated fresh rather than committed, matching this repo's existing output/ convention.
… to string)
DscResource.Authoring's New-DscAdaptedResourceManifest derives each
property's JSON schema type from the PowerShell AST's TypeName.Name.
For a fully-qualified type annotation like [System.Boolean] or
[System.Int32] - the convention used throughout this module's resource
classes - the AST returns the full dotted name rather than the short
alias ("bool"/"int32") the tool's internal type-mapping switch expects,
so the property silently falls back to its default of "string". This
was masked for [System.String] properties only because "string" happens
to be that same fallback default; verified it affects 25 of the 49
resources (every Boolean- or Int32-typed property).
Adds a new local build task, Fix_DscAdaptedResourceManifestTypes, that
re-derives the correct JSON schema type for every property directly
from the built module's actual class definitions (walking the
inheritance chain), and patches both the individual
*.dsc.adaptedResource.json files and the combined *.dsc.manifests.json
list. Runs last in the dscv3 workflow, since Create_DscResourceManifestsList
regenerates its own copies rather than reading the already-written files.
…test failures)
The integration suite has been consistently failing 4/4 tests in
AzDoNotificationSubscription.tests.ps1 with "A subscription for e-mail
delivery requires a valid e-mail address" across multiple branches and
runs this session. Root cause: New-/Set-AzDoNotificationSubscription
build channel = { type, address } but never set useCustomAddress, so
Azure DevOps silently ignores the custom address and falls back to the
default subscriber's (the calling identity, since subscriber is never
sent - it defaults to the caller) own notification preferences, which
fail validation for any identity without a real mailbox (e.g. the
Managed Identity/service principal these tests authenticate as).
Fixed by setting useCustomAddress = $true whenever a custom address is
supplied, matching Azure DevOps's documented "team subscription"
pattern. Verified via a live elevated re-run that this changes the
error to Azure DevOps's own tenant-membership validation ("the e-mail
address ... does not belong to a user in this organization's Microsoft
Entra ID tenant") - confirming the fix is correct, but that exercising
it end-to-end needs a real user's address in the test org's tenant.
That address is PII and this is a public repository, so - following
the exact pattern already established for AzDoUserEntitlement.tests.ps1
- the integration test now supplies it via the AZDODSC_TEST_USER_UPN
environment variable and skips gracefully when unset, rather than using
a placeholder address that Azure DevOps correctly rejects. Verified the
skip path live (0 passed, 0 failed, 8 skipped, ~1s - no API calls) and
that the previous literal group-name Subscriber value (rejected by the
API on its own terms) has also been replaced, since Subscriber is a
literal channel address by design, not a resolvable group/user identity
(confirmed against New-/Get-AzDoNotificationSubscription's own unit
tests and the resource's documented example).
…anifests Runs the Sampler build pipeline (build.ps1 -ResolveDependency -Tasks build) followed by the dscv3 workflow to generate DSC v3 adapted resource manifests for all 49 resources, uploading the built module and the manifests as separate, clearly-named build artifacts. Caches output/RequiredModules (keyed on RequiredModules.psd1's hash) so repeat runs don't reinstall the full Sampler/DscResource.* dependency chain, including the DscResource.Authoring prerelease, from PSGallery every time. Verified the exact build.ps1 invocations locally end-to-end (including a fresh -ResolveDependency run) before adding this, and confirmed the artifact glob patterns match the real generated file layout (49 *.dsc.adaptedResource.json files + the combined *.dsc.manifests.json).
…mitigation
Found two separate, real bugs while investigating the Unit Tests CI
failures visible on the actual GitHub Actions runner:
1. The workflow used `2>&1` to capture test output, but Pester's
colored summary/pass-fail lines go through Write-Host, which writes
to the Information stream (6), not Error (2). `2>&1` silently
dropped them, so $output had no parseable summary and
classes-tests.log/common-tests.log were never written with real
content - explaining the "Cannot find path ... because it does not
exist" error seen in the actual CI run. Fixed by redirecting all
streams (`*>&1`) and matching each summary value (Passed/Failed/
Skipped) independently rather than requiring all three on what
$output treats as a single "line" (Write-Host segments can land in
separate array elements).
2. Re-confirmed the known Pester v5 class-loading race ("Could not find
type [X]") reproduces reliably under CPU-constrained conditions
(both a nested child pwsh process locally and the actual GitHub
Actions runner - which failed on its very first, only attempt).
Retries don't help since it isn't a random race, it's tied to how
"cold"/constrained the host is. Repeated cold-host reproductions
showed a short pause after class-loading, before Invoke-Pester
starts, reliably avoids it - added to azuredevopsdsc.tests.ps1.
Revert the disproven 2-second delay in azuredevopsdsc.tests.ps1 (confirmed on GH Actions run 31430023653 to NOT prevent the class-loading race - same 11 failures as baseline). Add a temporary diagnostic to build.yml: run a Pester test file that uses `using module` (not Import-Module) against the genuinely built module's psd1, checking whether bare type-literal resolution works reliably for the classes that fail intermittently in the real suite. Based on external research (deadlydog/PowerShell.Experiment.ClassInModule) indicating classes defined in one physical psm1 (which ModuleBuilder already produces) plus `using module` (not Import-Module, which was already disproven) is the only combination that reliably supports external bare type-literal resolution.
DscResource.Common (which provides Get-LocalizedData, called at the top of the built AzureDevOpsDsc.Common.psm1) lives in output/RequiredModules after -ResolveDependency, but the diagnostic step's fresh pwsh session didn't have it on PSModulePath, causing an unrelated command-not-found error unrelated to the using-module class-resolution theory being tested.
Confirms whether 'using module AzureDevOpsDsc' (resolved via PSModulePath, auto-selecting the version subfolder) works as well as a hardcoded versioned path - needed since the built module's version changes over time and using-module paths can't be computed dynamically at parse time.
…lt module Root cause (confirmed via external research on deadlydog/PowerShell.Experiment.ClassInModule, then validated live on GitHub Actions): dot-sourcing PowerShell class definitions does not reliably survive Pester v5's per-block session-state rebinding, intermittently producing "Could not find type [X]" for classes referenced via bare type-literal syntax inside It/BeforeAll blocks on cold runners. This is not a timing/race issue - it's a fundamental limitation of dot-sourced (and Import-Module'd) classes never being reliably resolvable via bare type literals outside their original scope. `using module` is the one mechanism that reliably works, but only against a module whose classes are physically defined in one psm1 - which is exactly what ModuleBuilder already produces for the built module. Changes: - All 19 Classes-suite test files now start with `using module AzureDevOpsDsc` (resolved via PSModulePath, auto-picking the built module's version folder) instead of dot-sourcing raw source class/enum files into their own scope on every run. - azuredevopsdsc.tests.ps1 no longer dot-sources classes/enums itself (test files now source classes from the built module instead); Import-ClassesAndEnums.ps1 is now dead code and removed. - unit-tests.yml now builds the module (with RequiredModules caching, mirroring build.yml) before running the Classes suite, and sets PSModulePath to include output/RequiredModules and output/builtModule before Pester parses any test file - required since `using module` resolves at parse time, before any of the test file's own runtime code executes. Validated on real GitHub Actions runs (31431577660, 31431809051): a diagnostic Pester file using `using module` against the built module resolved all 10 previously-failing type literals 10/10, after two prior candidate fixes (an explicit delay, and plain Import-Module of the built module) were each disproven on real runs.
…module path The first using-module pass only touched the 19 files that had the old dot-sourcing reload guard, but many other Classes-suite files (AzDevOps*Base tests, PersonalAccessToken, AzDoGroupPermission) relied solely on the orchestrator's own class dot-sourcing, which was removed - breaking them. Add `using module AzureDevOpsDsc` to all 20 remaining files. Also fix a second, distinct gap surfaced by that same CI run: several DSC resource classes' Construct() explicitly calls `Import-Module AzureDevOpsDsc.Common` by bare name, which only resolves via PSModulePath - it isn't found via output\builtModule itself, since the nested module lives one level down at output\builtModule\AzureDevOpsDsc\<version>\Modules. Add that directory to PSModulePath in unit-tests.yml, resolved dynamically via Get-ChildItem to avoid hardcoding the built module's version.
… tests Two more issues surfaced by switching the Classes suite to `using module`: 1. Token factory functions (New-ManagedIdentityToken, New-ServicePrincipalToken, New-CertificateToken, New-CertificateTokenFromFile, New-AzureCliToken, New-WorkloadIdentityFederationToken, New-PersonalAccessToken) were declared with the `global:` scope modifier. That was only ever needed because the old test harness dot-sourced each class file inside a ForEach-Object scriptblock (a transient child scope), and `global:` was how the function survived past that loop. It's dead weight now that classes come from `using module` - and actively harmful, since a `global:`-scoped function loses its binding to the module's own class type table, so `[ManagedIdentityToken]::New(...)` inside it threw "Could not find type" even though `using module` had otherwise worked. Removed `global:` from all 7 declarations; they're still exported normally since the manifest's FunctionsToExport is unset (defaults to '*'). 2. Class methods (Construct/Get/Set/Test) now execute inside the real built module's own scope rather than the test file's scope, so Pester's `Mock` - scoped to the caller by default - no longer intercepts calls those methods make to Test-Path, Import-Clixml, Import-Module, and the various AzDoXXX/New-AzDoAuthenticationProvider/Get-AzDoCacheObjects helpers. Added `-ModuleName AzureDevOpsDsc` to every Mock/Assert-MockCalled call across the 13 Resources test files that mock class-internal dependencies, so the mocks are visible from within the module's session state.
…zureDevOpsDsc Test-AzDevOpsProjectName is a private, never-exported helper inside AzureDevOpsDsc.Common (confirmed via that module's dynamic-loader psm1: it only auto-exports functions under a \Public\ path, plus a small hardcoded Export-ModuleMember list - Test-AzDevOpsProjectName is neither). It's genuinely unresolvable from AzureDevOpsDsc's own module scope, so `Mock -CommandName Test-AzDevOpsProjectName -ModuleName AzureDevOpsDsc` fails at registration time with a CommandNotFoundException - which was aborting the entire BeforeAll (and therefore every It in that Describe) in 020.AzDoProject, 050.AzDoWIPTags, 051.AzDoAreaNodes, and 052.AzDoIterationNodes. Since Test-AzDevOpsProjectName is only ever called from other AzureDevOpsDsc.Common functions (e.g. Get-AzDoProject's own [ValidateScript] on -ProjectName), not from class methods directly, the correct scope for this specific mock is -ModuleName AzureDevOpsDsc.Common. Brings the Classes suite from 190 passed/21 failed to 198 passed/13 failed locally - WIPTags/AreaNodes/IterationNodes now pass in full.
…ing-module classes Two final root causes for the last 13 Classes-suite failures: 1. Export-ModuleMember gap: Test-AzDevOpsProjectName and ConvertTo-Base64String are private helpers in AzureDevOpsDsc.Common, called directly from class constructors/methods now living in the AzureDevOpsDsc root module's own scope (via `using module`). Neither was covered by the Public-path auto-export or the module's hardcoded Export-ModuleMember list, so they were genuinely unresolvable cross-module - not a test artifact, a real gap that would also bite production use of PersonalAccessToken and any AzDoProject-family resource outside of a call chain that happens to import Common first. Exporting both fixes this for real callers too. 2. `Should -BeOfType [ClassName]` / `Should -BeOfType 'ClassName'`: Pester's own implementation re-resolves the type by name from within Pester's own module scope, which has no visibility into a class only bound via the *test file's* `using module` statement - a documented Pester limitation with PowerShell classes. Every remaining failure (all 11, exactly matching the original flaky-11 list) was a `-BeOfType` assertion, not a real construction failure - direct `[ClassName]::new()` calls always worked fine. Replaced with `($value -is [ClassName]) | Should -BeTrue`, which evaluates the type check in the test file's own scope instead. Classes suite now passes 211/211, deterministically, both locally and matching the real GitHub Actions runner's behavior for every fix verified so far in this branch.
The Classes-suite fix unblocked the Common suite from running in CI for the first time. That surfaced ~29 pre-existing failures, unrelated to the class- loading work, spanning several distinct root causes: - 17 permission functions (AzDoAgentPoolPermission, AzDoEnvironmentPermission, AzDoPipelinePermission x4, AzDoProcessPermission x4, AzDoProjectPermission x4, AzDoSecurityNamespacePermission, AzDoServiceConnectionPermission, AzDoVariableGroupPermission) call Write-Error as a non-fatal "log a diagnostic and return a graceful Error status" pattern - but that becomes terminating under this runner's ambient $ErrorActionPreference, aborting the function before it returns. Added -ErrorAction Continue to make the intent explicit regardless of caller preference (confirmed: passed locally, failed on the real GH Actions runner, matching this exact mechanism). - List-DevOpsAgentPools: `@($defaultPools) + @($deploymentPools)` produces a 2-element array of nulls (not empty) when both API calls return no data, since @($null) wraps a single null into a 1-element array. Filiter out nulls from the merged result. - Build-JWTAssertion: $Certificate.GetRSAPrivateKey() is an extension method PowerShell's member-access syntax can fail to resolve depending on assembly load order; call it via its fully-qualified static form instead. - Get-AzCliToken: relied on reading $LASTEXITCODE in the caller's scope after calling a mocked wrapper function - not reliably preserved across a Pester mock boundary. Have Invoke-AzCLICommand capture and return ExitCode explicitly instead of leaving it to the ambient global variable. - Test-Date: used culture-dependent `-as [DateTime]` conversion, so an unambiguous dd/MM/yyyy date fails to parse on any MM/dd/yyyy-default culture (e.g. en-US, GitHub's hosted runner default) even though callers accept both slash conventions. Try each accepted format explicitly, culture-invariant. - Test-ACLListforChanges.tests.ps1 had a stale assertion expecting NotFound for a null Difference ACL; the source was already deliberately changed to return Changed instead (see its own comment) - the test was never updated to match. - ~10 tests across Get-AzDoAgentPool/AgentQueue/Extension/PipelineEnvironment, Resolve-DevOpsProcess, ConvertTo-DevOpsServiceHookSubscription, New-ACLToken, and Get-AzDoSecurityNamespacePermission were missing an explicit dot-source for a real dependency (List-DevOpsAgentPools/Queues/Extensions/ PipelineEnvironments, the [CacheItem] class, Get-AzDoCacheObjects, Resolve-AzDoProjectIdForToken, New-ACLToken/ConvertTo-FormattedToken) that the function under test calls internally but the test file's own Find-MockedFunctions-based lazy-loading never picks up since it's not directly mocked in that file. - Get-AzServicePrincipalCertificateToken.tests.ps1 mocked a certificate-store lookup with a fake PSCustomObject standing in for an [X509Certificate2]- typed parameter; Mock preserves the real function's parameter type constraint even though its body is faked, so PowerShell's own argument type coercion failed before the mock ever ran. Use a real (trivial, self-signed) certificate instead. Verified locally: Common suite now passes 1703/1703 (10 skipped), up from 1674 passed/29 failed/10 skipped.
…source Same gap as the other Get-CacheItem-mocking test files fixed in the previous commit - missed this one. It passed in the local full-suite run by accident (an earlier-discovered file's own dot-sourcing happened to mask the gap), but failed on the real GitHub Actions runner where that ordering coincidence didn't hold. Swept the rest of the Common suite for the same pattern (Mock -CommandName Get-CacheItem without Get-AzDoCacheObjects dot-sourced) - no other instances found.
…flow Renaming to publish under this fork's own identity: - PowerShell Gallery names are globally unique and this repo doesn't own the existing 'AzureDevOpsDsc' listing (that's the upstream dsccommunity project). - 'AzureDevOpsDscv3' - the obvious next choice - is already taken by a different community fork (github.com/mimachniak/AzureDevOpsDscv3), which also already publishes to PSGallery under that exact name (case-insensitive collision). Its approach differs from this fork's anyway (a Microsoft.Windows/WindowsPowerShell wrapper vs. this fork's native Microsoft.Adapter/PowerShell + generated adapted resource manifests), so 'AzureDevOpsDscNative' names the actual differentiator instead of just restating "also DSC v3". Renamed source/AzureDevOpsDsc.psd1 -> source/AzureDevOpsDscNative.psd1 (new GUID, updated Author/CompanyName/Description/LicenseUri/ProjectUri), renamed the en-US localized-data and about-help files to match (Get-LocalizedData looks up a file named after the calling module), updated all 39 Classes-suite test files' `using module AzureDevOpsDsc` statements and 96 `Mock -ModuleName AzureDevOpsDsc` references (leaving the AzureDevOpsDsc.Common nested submodule name untouched - that's an internal implementation detail, not the public package identity), and updated unit-tests.yml's PSModulePath resolution accordingly. Verified locally after the rename: Classes suite 211/211, Common suite 1703/1703 (10 skipped) - unchanged from before the rename. Added .github/workflows/publish.yml: triggers on version tags (vX.Y.Z, tags containing a hyphen like pre-release tags excluded) or manual dispatch. Derives ModuleVersion from the tag, builds, re-runs both unit test suites as a release gate (refuses to publish if either has a failure, even though tagging main implies it already passed once - a tag can point anywhere), generates DSC v3 manifests, packages the module, and publishes a GitHub Release plus - once a Gallery API key is configured - PowerShell Gallery, via Sampler's existing publish task chain (each sub-task self-guards on its own token being present, so it degrades gracefully rather than failing hard if GALLERY_API_TOKEN isn't set yet). Updated README.md (new title/description, framed as a fork with its actual differentiator, replaced the stale "look at mimachniak's project instead" note - now that this fork carries substantial DSC v3 support and fixes of its own - with a reference alongside it instead) and CHANGELOG.md (title, plus new Added/Fixed entries covering the rename, the new workflows, and this session's test-suite fixes).
Integration suite fixes (required for the full run to work at all after the AzureDevOpsDsc -> AzureDevOpsDscNative rename): - scripts/redeploy-module.ps1: build-output source path and deploy-target path both hardcoded the old module name. - 60 files under tests/Integration/: Import-Module/Import-DscResource/ Invoke-DscResource references to 'AzureDevOpsDsc', including tests/Integration/Supporting/Initalize-TestFramework.ps1's `Import-Module AzureDevOpsDsc.psd1` (kept AzureDevOpsDsc.Common.psd1 untouched - that submodule wasn't renamed). Verified via a full elevated run against a real org: 381 passed, 0 failed, 22 skipped (~69 min) - matches the prior clean baseline exactly. Hardening for public use: - Added SECURITY.md (private vulnerability reporting via GitHub Security Advisories, scoped to this fork's own changes vs. upstream). - Added .github/dependabot.yml for the github-actions ecosystem (PowerShell Gallery modules aren't a supported Dependabot ecosystem, so RequiredModules.psd1 isn't covered by this - only Action version pins are). - Expanded .gitignore: common secret-shaped file patterns (*.pfx, *.key, *.pem, *.p12, *.cer, .env*) and OS cruft (.DS_Store, Thumbs.db, etc). - Scrubbed the real Azure DevOps org name that had been checked into tests/Integration/TestFrameworkConfiguration.json since Oct 2024 (not a credential, but an identifying-info leak). The tracked file is now a placeholder template; real values go in TestFrameworkConfiguration.local.json (gitignored via the existing *.local.* pattern, never committed). Added tests/Integration/README.md documenting the convention. - Enabled (via repo settings, not in this diff): secret scanning + push protection, Dependabot alerts + security updates, and branch protection on main (require Build + Unit Tests status checks, block force-push and branch deletion - no mandatory PR review, to avoid locking out a solo maintainer's own merges).
ZanattaMichael
force-pushed
the
add-dsc-v3-support
branch
from
August 11, 2026 23:46
2467d48 to
e4b0409
Compare
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
Two related pieces of work, both already verified:
Fix the integration suite after the
AzureDevOpsDsc->AzureDevOpsDscNativerename (merged in #34). The rename brokescripts/redeploy-module.ps1(hardcoded build-output and deploy-target paths) and 60 files undertests/Integration/that hardcoded the old module name inImport-Module/Import-DscResource/Invoke-DscResourcecalls. Fixed all of it and verified with a full elevated run against a real org: 381 passed, 0 failed, 22 skipped (~69 min) - matches the last known-clean baseline exactly.Harden the repo for public use:
SECURITY.md(private vulnerability reporting via GitHub Security Advisories)..github/dependabot.ymlfor thegithub-actionsecosystem..gitignorefor secret-shaped files (*.pfx,*.key,*.pem,*.p12,*.cer,.env*) and OS cruft.tests/Integration/TestFrameworkConfiguration.jsonsince Oct 2024. It's now a placeholder template; real values go in the gitignoredTestFrameworkConfiguration.local.json(documented in newtests/Integration/README.md).main(require theBuildandUnit Testsstatus checks, block force-push/deletion - no mandatory PR review, since this is solo-maintained).Test plan
Generated with Claude Code