Skip to content

Address auth ALC review comments - #3635

Merged
Ramses Sanchez-Hernandez (ramsessanchez) merged 1 commit into
microsoftgraph:fixauth-module-assembly-loadingfrom
PrzemyslawKlys:codex/auth-alc-test-flakiness
Aug 12, 2026
Merged

Address auth ALC review comments#3635
Ramses Sanchez-Hernandez (ramsessanchez) merged 1 commit into
microsoftgraph:fixauth-module-assembly-loadingfrom
PrzemyslawKlys:codex/auth-alc-test-flakiness

Conversation

@PrzemyslawKlys

@PrzemyslawKlys Przemysław Kłys (PrzemyslawKlys) commented Jun 5, 2026

Copy link
Copy Markdown

Changes proposed in this pull request

  • Guard assemblyName.Name before using it in the managed ALC resolver.
  • Use Import-Module -LiteralPath for the Desktop/no-ALC fallback path.
  • Relax the worker-thread dependency resolution test so it does not assume Azure.Core is absent from AssemblyLoadContext.Default.

Why

This addresses the inline Copilot review comments from #3633 and the earlier test flakiness concern from #3632.

The production resolver should treat a missing assembly name as an ordinary resolution miss and return null. The Desktop fallback should import the exact DLL path instead of allowing wildcard interpretation. The test only needs to prove that the dependency resolved by the worker thread lands in the same non-default Graph authentication ALC; it should not fail merely because another test loaded Azure.Core in the default context.

Validation

  • pwsh -NoProfile -Command 'Invoke-Pester .\src\Authentication\Authentication\test\Microsoft.Graph.Authentication.Tests.ps1 -Output Detailed'
    • 8 tests passed
  • PowerShell parser validation for Microsoft.Graph.Authentication.psm1
  • git diff --check

Other links

@PrzemyslawKlys Przemysław Kłys (PrzemyslawKlys) changed the title Relax isolated dependency test assumption Address auth ALC review comments Jun 5, 2026
@ramsessanchez
Ramses Sanchez-Hernandez (ramsessanchez) merged commit 9506884 into microsoftgraph:fixauth-module-assembly-loading Aug 12, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants