refactor(scripts): standardize entry point guard pattern #346
Conversation
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
- Add Invoke-* orchestration functions to all scripts - Apply guard pattern: $MyInvocation.InvocationName -ne '.' - Document pattern in scripts/README.md - Simplify test dot-sourcing (removes AST workaround) - Scripts: 12 files updated across security, linting, extension
|
@azurecloudkevin please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #346 +/- ##
==========================================
+ Coverage 41.08% 42.87% +1.79%
==========================================
Files 15 15
Lines 2870 2904 +34
==========================================
+ Hits 1179 1245 +66
+ Misses 1691 1659 -32
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR standardizes a PowerShell script entry-point pattern across the repo, adding Invoke-* orchestration functions and a guard that distinguishes direct invocation from dot-sourcing, plus documentation for the pattern. The changes improve testability, make exit-code behavior more consistent, and clean up the SHA staleness tests by removing the AST-based workaround.
Changes:
- Added
Invoke-*orchestration functions and$MyInvocation.InvocationName -ne '.'guards to security, linting, and extension scripts, converting in-scriptexitcalls into integer return codes propagated by the guard. - Simplified
Test-SHAStalenessPester tests by switching from AST-based function extraction to simple dot-sourcing now that scripts can be safely imported without executing main logic. - Documented the standard entry-point pattern and naming conventions for scripts in
scripts/README.md.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
scripts/tests/security/Test-SHAStaleness.Tests.ps1 |
Updates tests to dot-source Test-SHAStaleness.ps1 directly, relying on the new guard pattern to prevent main execution during testing. |
scripts/security/Update-ActionSHAPinning.ps1 |
Introduces Invoke-ActionSHAUpdate as the main orchestration function, returns integer exit codes, and guards main execution behind the $MyInvocation.InvocationName -ne '.' check. |
scripts/security/Test-SHAStaleness.ps1 |
Adds Invoke-SHAStalenessTest orchestration, converts inline exits to integer returns, and uses a guarded main section that calls the orchestrator and exits with its code. |
scripts/security/Test-DependencyPinning.ps1 |
Extracts the main dependency-pinning logic into Invoke-DependencyPinningTest, replaces direct exits with returns, and adds a guard so dot-sourcing no longer fails the script. |
scripts/linting/Markdown-Link-Check.ps1 |
Wraps existing markdown link checking logic in Invoke-MarkdownLinkCheck with integer return codes and a guarded main execution path. |
scripts/linting/Link-Lang-Check.ps1 |
Adds Invoke-LinkLanguageCheck as the orchestration function, returning 0 consistently while the top-level guard handles exit behavior. |
scripts/linting/Invoke-YamlLint.ps1 |
Refactors main YAML linting logic into Invoke-YamlLintValidation with structured return codes and creates a guarded main section that exits with the function’s result. |
scripts/linting/Invoke-PSScriptAnalyzer.ps1 |
Moves PSScriptAnalyzer execution into Invoke-PSScriptAnalysis, returning an integer summary result and wiring it through a guarded main block. |
scripts/linting/Invoke-LinkLanguageCheck.ps1 |
Wraps Link-Lang-Check.ps1 with Invoke-LinkLanguageCheckWrapper, returning explicit exit codes and guarding the main entry for dot-sourcing scenarios. |
scripts/extension/Prepare-Extension.ps1 |
Extracts the extension preparation workflow into Invoke-ExtensionPreparation, uses $PSScriptRoot for path resolution, and returns integer codes that the guarded main section converts into process exits. |
scripts/extension/Package-Extension.ps1 |
Refactors packaging logic into Invoke-ExtensionPackaging, ensures all paths return explicit exit codes, and adds a guard-based main entry that calls the orchestrator and exits accordingly. |
scripts/README.md |
Documents the new script entry-point pattern, including structure, guard usage, and naming conventions, with a minor inconsistency in the naming table noted separately. |
scripts/README.md
Outdated
| | `Package-*.ps1` | `Invoke-*Packaging` | | ||
| | `Prepare-*.ps1` | `Invoke-*Preparation` | | ||
| | `Update-*.ps1` | `Invoke-*Update` | | ||
| | `Validate-*.ps1` | `Test-*Validation` | |
There was a problem hiding this comment.
In the naming convention table, the last row lists Test-*Validation as the "Invoke Function" for Validate-*.ps1 scripts, which is inconsistent with the preceding description that orchestration entrypoints follow the Invoke-* pattern and with new functions like Invoke-YamlLintValidation. To avoid confusion for future contributors, this row should be updated to use an Invoke-*Validation name to match the documented pattern and actual script implementations.
Suggested change
| | `Validate-*.ps1` | `Test-*Validation` | | |
| | `Validate-*.ps1` | `Invoke-*Validation` | |
Author
I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
|
📝 - Generated by Copilot
Add comprehensive test coverage improvements: P1 - Entry point tests: - Update-ActionSHAPinning: Invoke-ActionSHAUpdate tests - Markdown-Link-Check: Invoke-MarkdownLinkCheck tests P2 - Error path and boundary tests: - Package-Extension: null manifest, empty hashtable, edge cases - Prepare-Extension: malformed YAML, missing frontmatter handling - Test-DependencyPinning: empty inputs, SHA format validation - Test-SHAStaleness: version comparison edge cases, token validation - Link-Lang-Check: fix flaky test Coverage: 36.17% -> 53.71% (+17.54pp) Tests: 742 -> 766 (+24 tests, 760 passing)
| .DESCRIPTION | ||
| Coordinates staleness checking for GitHub Actions and tools, then outputs results. | ||
| .PARAMETER OutputFormat | ||
| Output format: 'json', 'azdo', 'github', or 'console'. |
There was a problem hiding this comment.
.PARAMETER OutputFormat documentation here only lists json, azdo, github, and console, but the parameter's ValidateSet also accepts BuildWarning and Summary, so the comment is out of date with the actual supported values. Please update the parameter description to reflect the full set of allowed formats so that callers using this function directly do not miss the additional options.
Suggested change
| Output format: 'json', 'azdo', 'github', or 'console'. | |
| Output format: 'json', 'azdo', 'github', 'console', 'BuildWarning', or 'Summary'. |
Comment on lines
+59
to
+69
| $repoRoot = git rev-parse --show-toplevel 2>$null | ||
| if ($LASTEXITCODE -ne 0) { | ||
| Write-Error "Not in a git repository" | ||
| return 1 | ||
| } | ||
|
|
||
| # Create logs directory if it doesn't exist | ||
| $logsDir = Join-Path $repoRoot "logs" | ||
| if (-not (Test-Path $logsDir)) { | ||
| New-Item -ItemType Directory -Path $logsDir -Force | Out-Null | ||
| } |
There was a problem hiding this comment.
The logic to resolve the git repository root and ensure a logs directory exists is now duplicated both in the top-level script (lines 17–28) and again inside Invoke-LinkLanguageCheckWrapper (lines 59–69). This duplication makes future changes to log location or git detection easy to forget in one place; consider centralizing this into a helper function or keeping it only in the wrapper so that behavior stays consistent and easier to maintain.
Add comprehensive tests for: - Package-Extension.ps1: version validation, parameter handling - Prepare-Extension.ps1: frontmatter parsing, discovery functions - Invoke-LinkLanguageCheck.ps1: wrapper function, output handling - Markdown-Link-Check.ps1: extended path handling, CLI scenarios Fix zero-coverage issue in 4 test files by replacing AST parsing with direct dot-sourcing for proper coverage tracking. Coverage improvement: 53.71% -> 63.98% (+10.27pp)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Pull Request
Description
This PR standardizes the "direct invocation vs dot-sourced" guard pattern across all PowerShell scripts in the repository. This improves testability by allowing scripts to be dot-sourced without executing their main logic, enabling Pester tests to directly call individual functions.
Related Issue(s)
Closes issue 325
Type of Change
Select all that apply:
Code & Documentation:
Infrastructure & Configuration:
AI Artifacts:
prompt-builderagent and addressed all feedback.github/instructions/*.instructions.md).github/prompts/*.prompt.md).github/agents/*.agent.md)Other:
.ps1,.sh,.py)Testing
Checklist
Required Checks
AI Artifact Contributions
/prompt-analyzeto review contributionprompt-builderreviewRequired Automated Checks
The following validation commands must pass before merging:
npm run lint:mdnpm run spell-checknpm run lint:frontmatternpm run lint:md-linksnpm run lint:psSecurity Considerations
Additional Notes