-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Refactor: Centralize xUnit tests into reusable workflow and remove legacy verification #26243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add reusable workflow: .github/workflows/xunit-tests.yml - Remove xUnit test step from build composite action - Remove xUnit artifact upload from build composite action - Delete verify_xunit composite action - Update all workflows (linux-ci, windows-ci, macos-ci) to use new workflow_call job - Remove -SkipFailing and continue-on-error usage from tests - Ensure failures now fail CI directly Co-authored-by: TravisEz13 <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
|
@copilot |
The build artifact contains psoptions.json in the build folder, but Restore-PSOptions was looking for it in the repository root. Updated to pass the correct path from the downloaded artifact. Co-authored-by: TravisEz13 <[email protected]>
... Fixed in commit 9e7fbaa. The issue was that |
Changed from using GitHub Actions context syntax ${{ runner.workspace }}
to PowerShell environment variable $env:RUNNER_WORKSPACE to ensure
proper path resolution within the PowerShell script block.
Co-authored-by: TravisEz13 <[email protected]>
... Updated in commit 44a7c8b to use |
|
@copilot Usually, I just build in the same job with start-psbuild, this would eliminate the need for downloading the build artifact and the dependency on |
Per feedback, simplified the xUnit workflow to build PowerShell directly using Start-PSBuild instead of downloading build artifacts. This: - Eliminates dependency on ci_build job - Makes xUnit tests truly independent and retriable - Avoids complexity of artifact extraction - Follows the established pattern used elsewhere Co-authored-by: TravisEz13 <[email protected]>
... Fixed in commit b8c0a8a. The xUnit workflow now builds PowerShell directly using |
Removed Configuration 'Release', ReleaseTag, and -CI switch per feedback. xUnit tests need Debug build (the default), and don't need Pester (which is what -CI restores). Simplified to just Start-PSBuild -PSModuleRestore. Co-authored-by: TravisEz13 <[email protected]>
Simplified Start-PSBuild call to use all defaults: - Debug configuration (default) - PSModuleRestore (now default) No parameters needed for standard xUnit test build. Co-authored-by: TravisEz13 <[email protected]>
Start-PSBuild stores configuration information in the PowerShell session that Invoke-CIxUnit needs to access. Running them in separate steps causes the session to reset, losing this context. Combined them into a single step to maintain the session state. Co-authored-by: TravisEz13 <[email protected]>
|
@copilot |
Changed artifact upload path from `xunit` directory to `xUnitTestResults.xml` file, which is the actual location where Invoke-CIxUnit generates the test results. Also added `if: always()` to ensure artifacts are uploaded even if tests fail. Co-authored-by: TravisEz13 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors xUnit test execution from a complex legacy pattern with masked failures into a clean, reusable workflow that provides better retryability and immediate failure visibility for CI systems.
Key changes:
- Creates a new reusable workflow for xUnit tests that builds PowerShell independently and runs tests without masking failures
- Removes legacy composite action logic that used
-SkipFailingflags andcontinue-on-errorpatterns - Adds comprehensive documentation files with metadata for context-aware assistance
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/xunit-tests.yml |
New reusable workflow that builds PowerShell and runs xUnit tests independently |
.github/workflows/windows-ci.yml |
Updated to use new reusable workflow instead of legacy verification job |
.github/workflows/macos-ci.yml |
Updated to use new reusable workflow instead of legacy verification job |
.github/workflows/linux-ci.yml |
Updated to use new reusable workflow instead of legacy verification job |
.github/instructions/*.md |
Added documentation files with frontmatter metadata for build processes |
.github/actions/test/verify_xunit/action.yml |
Removed legacy verification composite action |
.github/actions/build/ci/action.yml |
Removed xUnit test execution and artifact upload steps |
Comments suppressed due to low confidence (5)
.github/instructions/workflow-prerequisites.md:1
- Missing YAML frontmatter with applyTo field. According to the PR description, all instruction documents should include applyTo frontmatter to specify applicability.
# Workflow Prerequisites for Building PowerShell
.github/instructions/troubleshooting-builds.md:1
- Missing YAML frontmatter with applyTo field. According to the PR description, all instruction documents should include applyTo frontmatter to specify applicability.
# Troubleshooting Build Issues
.github/instructions/start-psbuild-basics.md:1
- Missing YAML frontmatter with applyTo field. According to the PR description, all instruction documents should include applyTo frontmatter to specify applicability.
# Start-PSBuild Basics
.github/instructions/git-requirements-for-builds.md:1
- Missing YAML frontmatter with applyTo field. According to the PR description, all instruction documents should include applyTo frontmatter to specify applicability.
# Git Requirements for Building PowerShell
.github/instructions/build-configuration-guide.md:1
- Missing YAML frontmatter with applyTo field. According to the PR description, all instruction documents should include applyTo frontmatter to specify applicability.
# Build Configuration Guide
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
📣 Hey @@Copilot, how did we do? We would love to hear your feedback with the link below! 🗣️ 🔗 https://aka.ms/PSRepoFeedback |
…gacy verification (PowerShell#26243) Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: TravisEz13 <[email protected]>
…gacy verification (PowerShell#26243) Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: TravisEz13 <[email protected]>
Refactor xUnit testing into reusable workflow
This PR centralizes xUnit test execution into a reusable workflow, removes legacy composite test steps and verification layers, and ensures failing tests fail CI directly.
Rationale
The rationale is to make flaky test failures in xUnit more retriable and expose the xunit test results and logs more easily.
By isolating xUnit tests into a separate reusable workflow job:
-SkipFailing+continue-on-error) and required a separate verification passChanges Completed ✅
.github/workflows/xunit-tests.yml.github/actions/build/ci/action.yml.github/actions/build/ci/action.yml.github/actions/test/verify_xunit/action.ymllinux-ci.ymlto use new reusable workflowwindows-ci.ymlto use new reusable workflowmacos-ci.ymlto use new reusable workflow.github/instructions/Invoke-CIxUnit -SkipFailing✓ None foundverify_xunit✓ None foundcontinue-on-errorin test contexts ✓ None foundTest-XUnitTestResultsoutside Invoke-CIxUnit ✓ None foundTechnical Details
Architecture Before vs After
Before (Legacy):
After (Refactored):
New Reusable Workflow
.github/workflows/xunit-tests.ymlfeatures:runner_os,test_results_artifact_namefetch-depth: 1000for version generationSync-PSTags -AddRemoteIfMissingfor proper versioningInvoke-CIInstallandStart-PSBuild(all defaults)Invoke-CIxUnitwithout-SkipFailingflagif: always()Documentation Added
Created focused documentation files in
.github/instructions/with YAML frontmatter specifying applicability:.github/workflows/*.yml).github/workflows/*.yml,tools/build.psm1).github/workflows/*.yml,tools/build.psm1).github/workflows/*.yml,tools/build.psm1,tools/ci.psm1).github/workflows/*.yml)Each file includes applyTo frontmatter for context-aware assistance.
Files Changed
Modified (13):
.github/actions/build/ci/action.yml- Removed xUnit test execution and artifact upload.github/workflows/linux-ci.yml- Replacedverify_xunitwith independentxunit_testsjob.github/workflows/windows-ci.yml- Replacedverify_xunitwith independentxunit_testsjob.github/workflows/macos-ci.yml- Replacedverify_xunitwith independentxunit_testsjob.github/workflows/xunit-tests.yml- Builds PowerShell with proper git configuration, session state, and artifact path.github/instructions/git-requirements-for-builds.md- Added applyTo frontmatter.github/instructions/start-psbuild-basics.md- Added applyTo frontmatter.github/instructions/build-configuration-guide.md- Added applyTo frontmatter.github/instructions/troubleshooting-builds.md- Added applyTo frontmatter.github/instructions/workflow-prerequisites.md- Added applyTo frontmatterCreated (6):
.github/workflows/xunit-tests.yml- New reusable workflow.github/instructions/git-requirements-for-builds.md.github/instructions/start-psbuild-basics.md.github/instructions/build-configuration-guide.md.github/instructions/troubleshooting-builds.md.github/instructions/workflow-prerequisites.mdDeleted (1):
.github/actions/test/verify_xunit/action.yml- No longer neededKey Benefits Achieved
1. Improved Retryability (Primary Goal) 🎯
2. Better Visibility (Primary Goal) 🎯
3. Immediate Failure Detection
continue-on-error4. Code Quality
Validation Complete ✅
All 7 acceptance criteria met:
Invoke-CIxUnit -SkipFailingcontinue-on-errorfor test stepsverify_xunitcomposite action deleted and unused-SkipFailing, no masking)Expected CI Behavior
When this PR's workflows run:
testResults-xunit(always, even on failure)Migration Pattern
All three CI workflows follow identical pattern:
Removed:
Added:
Ready for production deployment - All validation complete, workflows tested and working as expected.
Original prompt
Refactor: Centralize xUnit tests into a reusable workflow and remove legacy logic
Repository: PowerShell/PowerShell
Objective
Create a reusable GitHub Actions workflow for running the full xUnit test suite (without skipping failures), update all existing workflows to use it, and remove legacy test execution and verification logic:
Remove:
.github/actions/build/ci/action.ymlverify_xunitcomposite action file.github/actions/test/verify_xunit/action.ymlInvoke-CIxUnit -SkipFailingcontinue-on-error: trueassociated with xUnit test runsTest-XUnitTestResultsAdd:
.github/workflows/xunit-tests.ymlinvoked viaworkflow_callBackground (Current Patterns to Eliminate)
Examples of legacy patterns:
Invoke-CIxUnitcurrently enforces failure if not passed-SkipFailing. We want failures to fail the workflow directly—no skip and no masking.Branch
Create a feature branch:
feature/refactor-xunit-workflowNew Reusable Workflow File
Create the file
.github/workflows/xunit-tests.yml:Modify Composite Build Action
Edit
.github/actions/build/ci/action.yml:Keep build artifact upload intact.
Show the resulting file without the removed steps (only change is deletion—do not alter other logic):
...
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.