feat(skills): mandate unit testing and document language support#636
Merged
WilliamBerryiii merged 5 commits intomainfrom Feb 17, 2026
Merged
feat(skills): mandate unit testing and document language support#636WilliamBerryiii merged 5 commits intomainfrom
WilliamBerryiii merged 5 commits intomainfrom
Conversation
- Add unit testing requirements with 80% code coverage minimum - Add supported languages table (PowerShell, Python) - Extend Pester coverage config to discover skill scripts - Add tests/ exclusion to extension packaging - Add programming language dropdown to skill request template Closes #634 ✨ - Generated by Copilot
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #636 +/- ##
==========================================
- Coverage 85.48% 85.47% -0.02%
==========================================
Files 23 23
Lines 4541 4543 +2
==========================================
+ Hits 3882 3883 +1
- Misses 659 660 +1
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 establishes unit testing requirements for GitHub Copilot skills, mandating 80% code coverage with co-located test directories to keep skills self-contained. It documents supported programming languages (PowerShell with full CI, Python with planned CI), extends Pester coverage to include skill scripts, and ensures test directories are excluded from extension packaging.
Changes:
- Added "Untested Skills" rejection criterion requiring unit tests with 80% code coverage minimum
- Extended Pester configuration to discover and report on skill scripts from
.github/skills/for code coverage - Updated extension packaging to exclude co-located
tests/directories from VSIX packages - Added comprehensive Unit Testing Requirements and Supported Languages sections to skills documentation
- Added Testing checklist items and
npm run test:psto pre-submission validation workflow
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/tests/pester.config.ps1 | Extends coverage path resolution to include .github/skills/ directory with proper guards and array concatenation |
| scripts/extension/Package-Extension.ps1 | Adds removal of co-located tests/ directories after copying skill artifacts to extension package |
| docs/contributing/skills.md | Adds Unit Testing Requirements, Supported Languages table, Python test conventions, and Testing checklist section |
| docs/architecture/testing.md | Documents Skills Testing section covering co-located test pattern, coverage integration, and packaging exclusion |
| CONTRIBUTING.md | Adds paragraph explaining skill test co-location pattern with reference to Skills Guide |
| .github/ISSUE_TEMPLATE/skill-request.yml | Adds Programming Language dropdown field with PowerShell and Python options |
- add pyproject to cspell word list - fix table alignment in Supported Languages table - add Other option to skill-request language dropdown - clarify cross-platform requirement in dropdown description 🔧 - Generated by Copilot
agreaves-ms
approved these changes
Feb 17, 2026
- correct file structure diagrams to show scripts/ subdirectory - clarify test discovery requires explicit path inclusion - add Bash row to Supported Languages table - update skill-request template to mention Bash and PowerShell - narrow pester coverage scan to per-skill script collection 📝 - Generated by Copilot
| $skillScripts = Join-Path $skillRoot 'scripts' | ||
| $paths = @() | ||
|
|
||
| $paths += Get-ChildItem -Path $skillRoot -Include '*.ps1', '*.psm1' -File -ErrorAction SilentlyContinue |
There was a problem hiding this comment.
The -Include parameter on this line requires a -Recurse or -Filter parameter to work correctly with Get-ChildItem. Without -Recurse, -Include is ignored when enumerating files in a single directory.
Since this line is enumerating files directly in $skillRoot (not recursively), the -Include parameter will have no effect. Scripts at the skill root level (e.g., .github/skills/video-to-gif/convert.ps1) will not be discovered.
Either:
- Remove the
-Includeparameter and use-Filter '*.ps1' | Where-Object { $_.Extension -in '.ps1', '.psm1' } - Add the
-Recurseparameter (though this would also include scripts in subdirectories, which may not be intended) - Use two separate
Get-ChildItemcalls with-Filter '*.ps1'and-Filter '*.psm1'
Note that line 73 correctly uses -Include with -Recurse for the scripts subdirectory.
Suggested change
| $paths += Get-ChildItem -Path $skillRoot -Include '*.ps1', '*.psm1' -File -ErrorAction SilentlyContinue | |
| $paths += Get-ChildItem -Path $skillRoot -File -ErrorAction SilentlyContinue | | |
| Where-Object { $_.Extension -in '.ps1', '.psm1' } |
Comment on lines
+191
to
+198
| Coverage path resolution for skills uses the repository root rather than `$scriptRoot` (which points to `scripts/`): | ||
|
|
||
| ```powershell | ||
| $repoRoot = Split-Path $scriptRoot -Parent | ||
| $skillScripts = Get-ChildItem -Path (Join-Path $repoRoot '.github/skills') ` | ||
| -Include '*.ps1', '*.psm1' -Recurse -File -ErrorAction SilentlyContinue | | ||
| Where-Object { $_.FullName -notmatch '\.Tests\.ps1$' } | ||
| ``` |
There was a problem hiding this comment.
The code example shown here is a simplified version that doesn't accurately represent the actual implementation in scripts/tests/pester.config.ps1. The actual implementation:
- Iterates through skill directories and checks both the skill root and the
scripts/subdirectory separately - Uses a
$pathsarray to accumulate results before filtering - Applies the test file filter (
-notmatch '\.Tests\.ps1$') after collecting all paths
The example implies a single recursive search with inline filtering, which doesn't match the actual logic. Consider either:
- Showing the actual code block from pester.config.ps1 (lines 65-78)
- Adding a note that this is a conceptual example, not the literal implementation
- Simplifying the example further to just explain the concept without showing misleading code
Suggested change
| Coverage path resolution for skills uses the repository root rather than `$scriptRoot` (which points to `scripts/`): | |
| ```powershell | |
| $repoRoot = Split-Path $scriptRoot -Parent | |
| $skillScripts = Get-ChildItem -Path (Join-Path $repoRoot '.github/skills') ` | |
| -Include '*.ps1', '*.psm1' -Recurse -File -ErrorAction SilentlyContinue | | |
| Where-Object { $_.FullName -notmatch '\.Tests\.ps1$' } | |
| ``` | |
| Conceptually, coverage path resolution for skills uses the repository root rather than `$scriptRoot` (which points to `scripts/`): | |
| ```powershell | |
| # Conceptual example only; see scripts/tests/pester.config.ps1 for the full implementation. | |
| $repoRoot = Split-Path $scriptRoot -Parent | |
| $skillScripts = Get-ChildItem -Path (Join-Path $repoRoot '.github/skills') ` | |
| -Include '*.ps1', '*.psm1' -Recurse -File -ErrorAction SilentlyContinue | | |
| Where-Object { $_.FullName -notmatch '\.Tests\.ps1$' } |
Comment on lines
+567
to
+569
| # Remove co-located test directories from packaged skills | ||
| Get-ChildItem -Path $destPath -Directory -Filter 'tests' -Recurse -ErrorAction SilentlyContinue | | ||
| Remove-Item -Recurse -Force |
There was a problem hiding this comment.
The test directory removal happens after copying the skill to $destPath, but the logic assumes $destPath is a directory. Looking at line 565, Copy-Item with -Recurse copies the skill directory structure, so $destPath should indeed be a directory.
However, the Get-ChildItem -Path $destPath -Directory -Filter 'tests' -Recurse will find ALL tests directories within the skill, including nested ones. If a skill contains example projects or nested structures with their own tests directories, those would also be removed.
Consider either:
- Limiting the search to the immediate child level:
Get-ChildItem -Path $destPath -Directory -Filter 'tests' -Depth 1(requires PS 5.0+) - Adding a more specific filter based on the documented structure (e.g., only remove tests directories that are direct children of the skill root)
- Documenting in the Skills Guide that nested
testsdirectories in examples or subprojects will also be excluded from packaging
Suggested change
| # Remove co-located test directories from packaged skills | |
| Get-ChildItem -Path $destPath -Directory -Filter 'tests' -Recurse -ErrorAction SilentlyContinue | | |
| Remove-Item -Recurse -Force | |
| # Remove co-located root-level test directory from packaged skills | |
| $testsRoot = Join-Path -Path $destPath -ChildPath 'tests' | |
| if (Test-Path -LiteralPath $testsRoot -PathType Container) { | |
| Remove-Item -LiteralPath $testsRoot -Recurse -Force | |
| } |
Comment on lines
+314
to
+316
| | Bash | `.sh` | N/A | shellcheck | Lint only | | ||
| | PowerShell | `.ps1` | Pester 5.x | PSScriptAnalyzer | Full (lint + test) | | ||
| | Python | `.py` | pytest | ruff (line-length=88, target-version=py311) | Planned | |
There was a problem hiding this comment.
There is an inconsistency regarding Bash script testing requirements:
- Line 46 states that "Untested Skills" (lacking unit tests or failing to achieve 80% code coverage) will be rejected
- Line 266 states "All skill scripts MUST include unit tests that achieve a minimum of 80% code coverage"
- However, the Supported Languages table (line 314) lists Bash with "N/A" for Test Framework and "Lint only" for CI Coverage
- The Unit Testing Requirements section includes subsections for PowerShell Tests and Python Tests, but not for Bash Tests
- The issue template description (line 24 in skill-request.yml) states "All skills require Bash (macOS/Linux) and PowerShell (Windows) scripts"
This creates ambiguity: are Bash scripts exempt from the testing requirement, or is testing infrastructure simply not yet available? If Bash scripts are exempt, the wording at lines 46 and 266 should be clarified to specify "PowerShell and Python scripts" rather than "all skill scripts". If Bash scripts do require tests, the table should indicate the expected test framework (e.g., BATS, shunit2) and the Unit Testing Requirements section should include a Bash Tests subsection.
WilliamBerryiii
pushed a commit
that referenced
this pull request
Feb 20, 2026
🤖 I have created a release *beep* *boop* --- ## [3.0.0](hve-core-v2.3.10...hve-core-v3.0.0) (2026-02-20) ### ⚠ BREAKING CHANGES * **skills:** migrate PR reference generation to self-contained skill ([#669](#669)) * restructure RPI collection to HVE Core naming convention ([#668](#668)) ### ✨ Features * **agents:** add agile-coach agent ([#562](#562)) ([de8d86c](de8d86c)) * **agents:** add DT coach agent with tiered instruction loading ([#656](#656)) ([206d3a7](206d3a7)) * **agents:** add product manager advisor and UX/UI designer agents ([#627](#627)) ([539eb8a](539eb8a)) * **agents:** add system architecture reviewer for design trade-offs and ADR creation ([#626](#626)) ([de5cfd6](de5cfd6)) * **build:** pin devcontainer image and align tool parity ([#704](#704)) ([6258b1c](6258b1c)) * **design-thinking:** add manufacturing industry context template ([#682](#682)) ([ce864bf](ce864bf)) * **instructions:** add DT coaching state protocol for session persistence ([#654](#654)) ([5a5be4e](5a5be4e)) * **instructions:** add dt-coaching-identity ambient instruction ([#642](#642)) ([6209a0d](6209a0d)) * **instructions:** add dt-method-01-deep for advanced scope conversation techniques ([#673](#673)) ([cc92ef9](cc92ef9)) * **instructions:** add dt-method-03-deep for advanced input synthesis techniques ([#676](#676)) ([0079a4f](0079a4f)) * **instructions:** add dt-method-09-deep instructions for Method 9 advanced coaching ([#703](#703)) ([150b2a6](150b2a6)) * **instructions:** add dt-method-sequencing ambient instruction ([#650](#650)) ([e465b2f](e465b2f)) * **instructions:** add dt-quality-constraints and design-thinking collection ([#645](#645)) ([17002bd](17002bd)) * **instructions:** add DT-to-RPI handoff contract specification ([#679](#679)) ([87f9962](87f9962)) * **instructions:** add energy industry context template ([#687](#687)) ([41088d8](41088d8)) * **instructions:** add healthcare industry context template ([#686](#686)) ([b2d5281](b2d5281)) * **instructions:** add Method 1 Scope Conversations coaching knowledge ([#651](#651)) ([93e2d48](93e2d48)) * **instructions:** add Method 2 Design Research coaching knowledge ([#652](#652)) ([30f7f3b](30f7f3b)) * **instructions:** add Method 3 Input Synthesis coaching knowledge ([#653](#653)) ([1efdb7d](1efdb7d)) * **instructions:** add Method 7 High-Fidelity Prototypes coaching instruction ([#666](#666)) ([9233eab](9233eab)) * **instructions:** add pull request instructions for PR generation workflow ([#706](#706)) ([73d23eb](73d23eb)) * **instructions:** create DT curriculum content (9 modules) ([#690](#690)) ([9f7378f](9f7378f)), closes [#617](#617) * **instructions:** create dt-method-02-deep.instructions.md ([#700](#700)) ([4d4d0ca](4d4d0ca)) * **instructions:** create dt-method-06-lofi-prototypes.instructions.md ([#684](#684)) ([4d5f757](4d5f757)) * **instructions:** create dt-method-07-deep.instructions.md ([#678](#678)) ([d3ec70d](d3ec70d)) * **instructions:** Create dt-method-08-deep.instructions.md ([#683](#683)) ([d9e1115](d9e1115)) * **instructions:** create dt-method-08-testing.instructions.md ([#681](#681)) ([3008ad8](3008ad8)) * **instructions:** create dt-method-09-iteration.instructions.md ([#685](#685)) ([9d7f4f5](9d7f4f5)) * **instructions:** create dt-rpi-research-context.instructions.md ([#689](#689)) ([34c7b89](34c7b89)) * **instructions:** create manufacturing reference learning scenario ([#692](#692)) ([1bd3994](1bd3994)) * **instructions:** Design Thinking Method 4 brainstorming instruction file ([#664](#664)) ([06f90b0](06f90b0)) * **prompts:** add DT start-project prompt for coaching initialization ([#657](#657)) ([ce583d5](ce583d5)) * **prompts:** add dt-resume-coaching prompt for session recovery ([#665](#665)) ([11b93cb](11b93cb)) * **prompts:** create dt-handoff-problem-space.prompt.md ([#688](#688)) ([277963d](277963d)) * **scripts:** add collection-level maturity field with validation, gating, and notices ([#697](#697)) ([7b1c8e8](7b1c8e8)) * **scripts:** add per-violation CI annotations and colorized console output ([#637](#637)) ([bd7d512](bd7d512)) * **skills:** edit SKILL frontmatter schema, add CI validation, and documentation ([#625](#625)) ([0138a78](0138a78)) * **skills:** mandate unit testing and document language support ([#636](#636)) ([9263617](9263617)) * **skills:** migrate PR reference generation to self-contained skill ([#669](#669)) ([cf8805f](cf8805f)) ### 🐛 Bug Fixes * **collections:** migrate artifacts into collection-based subdirectories ([#658](#658)) ([dfa5261](dfa5261)) * **instructions:** optimize Phase 1 DT token budgets and close [#564](https://github.com/microsoft/hve-core/issues/564)/[#565](https://github.com/microsoft/hve-core/issues/565) gaps ([#675](#675)) ([4f42f00](4f42f00)) * **scripts:** add CI annotations and step summary to copyright header check ([#638](#638)) ([5fa6328](5fa6328)) * **scripts:** add grouped link-lang console diagnostics and failure summary ([#661](#661)) ([4d6871f](4d6871f)) * **scripts:** add per-violation Write-Host and Write-CIAnnotation output to Test-DependencyPinning ([#640](#640)) ([9d3b71d](9d3b71d)) * **scripts:** align agent frontmatter schema with VS Code spec ([#469](#469)) ([254d445](254d445)) * **scripts:** optimize PSScriptAnalyzer linting performance in WSL2 ([#667](#667)) ([f120b93](f120b93)) * **scripts:** stabilize YAML display key ordering in collection manifest ([#701](#701)) ([73c0d2c](73c0d2c)) * **scripts:** use text stubs for plugin links when symlinks unavailable ([#695](#695)) ([d7650a3](d7650a3)) * **skills:** fix powershell test coverage in pr-reference skill ([#699](#699)) ([408e6b7](408e6b7)) ### 📚 Documentation * **dt:** add Method 5 Concepts and Method 6 Lo-Fi Prototypes instructions ([#693](#693)) ([cfdcf11](cfdcf11)) * **hve-guide:** add role-based guides and project lifecycle documentation ([#663](#663)) ([17a85da](17a85da)) ### ♻️ Refactoring * restructure RPI collection to HVE Core naming convention ([#668](#668)) ([120dde0](120dde0)) * **scripts:** consolidate duplicate logging into shared SecurityHelpers module ([#655](#655)) ([627a877](627a877)) * **scripts:** use shared SecurityHelpers and CIHelpers modules in security scripts ([#705](#705)) ([3a0baa7](3a0baa7)) ### 🔧 Maintenance * **deps-dev:** bump markdownlint-cli2 from 0.20.0 to 0.21.0 in the npm-dependencies group ([#609](#609)) ([1486dd7](1486dd7)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: hve-core-release-please[bot] <254602402+hve-core-release-please[bot]@users.noreply.github.com>
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
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.
Description
Added unit testing requirements with an 80% code coverage minimum for skill contributions and documented supported programming languages. Tests use a co-located
tests/directory pattern inside each skill, keeping skills self-contained. Updated Pester coverage configuration and extension packaging to support the new test structure.npm run test:psto automated validationtests/subdirectorytests/directory exclusion from VSIX packaging inPackage-Extension.ps1.github/skills/docs/architecture/testing.mdcovering co-located test pattern, coverage integration, and packaging exclusionCONTRIBUTING.mdexplaining skill test co-location pattern with link to Skills Guidepyprojectto cspell word listRelated Issue(s)
Closes #634
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).github/skills/*/SKILL.md)Other:
.ps1,.sh,.py)Sample Prompts (for AI Artifact Contributions)
Testing
lint:md,lint:yaml,lint:frontmatter,lint:psLASTEXITCODE: -1leak from subprocess invocations is unrelated)Test-Pathguard without errorChecklist
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
🧪 - Generated by Copilot