Skip to content

Commit 9263617

Browse files
feat(skills): mandate unit testing and document language support (#636)
## 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. - feat(skills): added "Untested Skills" rejection criterion requiring unit tests with 80% code coverage - feat(skills): added Unit Testing Requirements section with test file layout, PowerShell and Python conventions, and packaging exclusion notes - feat(skills): added Supported Languages table documenting PowerShell (full CI) and Python (planned CI) with tooling expectations - feat(skills): added Testing checklist to the pre-submission verification section and `npm run test:ps` to automated validation - feat(skills): updated file structure diagram to include `tests/` subdirectory - feat(extension): added co-located `tests/` directory exclusion from VSIX packaging in `Package-Extension.ps1` - feat(scripts): extended Pester coverage configuration to discover and report on skill scripts from `.github/skills/` - docs(architecture): added Skills Testing section to `docs/architecture/testing.md` covering co-located test pattern, coverage integration, and packaging exclusion - docs(contributing): added paragraph in `CONTRIBUTING.md` explaining skill test co-location pattern with link to Skills Guide - feat(skills): added Programming Language dropdown (PowerShell, Python, Other) to skill request issue template with clarified cross-platform description - fix(skills): added `pyproject` to cspell word list - fix(skills): fixed table alignment in Supported Languages table - fix(templates): added "Other" option to skill request language dropdown to match documentation reference ## Related Issue(s) Closes #634 ## Type of Change Select all that apply: **Code & Documentation:** - [ ] Bug fix (non-breaking change fixing an issue) - [x] New feature (non-breaking change adding functionality) - [ ] Breaking change (fix or feature causing existing functionality to change) - [x] Documentation update **Infrastructure & Configuration:** - [ ] GitHub Actions workflow - [ ] Linting configuration (markdown, PowerShell, etc.) - [ ] Security configuration - [ ] DevContainer configuration - [ ] Dependency update **AI Artifacts:** - [ ] Reviewed contribution with `prompt-builder` agent and addressed all feedback - [ ] Copilot instructions (`.github/instructions/*.instructions.md`) - [ ] Copilot prompt (`.github/prompts/*.prompt.md`) - [ ] Copilot agent (`.github/agents/*.agent.md`) - [ ] Copilot skill (`.github/skills/*/SKILL.md`) > **Note for AI Artifact Contributors**: > > - **Agents**: Research, indexing/referencing other project (using standard VS Code GitHub Copilot/MCP tools), planning, and general implementation agents likely already exist. Review `.github/agents/` before creating new ones. > - **Skills**: Must include PowerShell scripts for cross-platform support. See [Skills](../docs/contributing/skills.md). > - **Model Versions**: Only contributions targeting the **latest Anthropic and OpenAI models** will be accepted. Older model versions (e.g., GPT-3.5, Claude 3) will be rejected. > - See [Agents Not Accepted](../docs/contributing/custom-agents.md#agents-not-accepted) and [Model Version Requirements](../docs/contributing/ai-artifacts-common.md#model-version-requirements). **Other:** - [x] Script/automation (`.ps1`, `.sh`, `.py`) - [ ] Other (please describe): ## Sample Prompts (for AI Artifact Contributions) ## Testing - All four linters pass clean: `lint:md`, `lint:yaml`, `lint:frontmatter`, `lint:ps` - All 228 Pester tests pass (pre-existing `LASTEXITCODE: -1` leak from subprocess invocations is unrelated) - No new skill scripts exist yet, so skill coverage path resolution executes the `Test-Path` guard without error ## Checklist ### Required Checks - [x] Documentation is updated (if applicable) - [x] Files follow existing naming conventions - [x] Changes are backwards compatible (if applicable) - [x] Tests added for new functionality (if applicable) ### AI Artifact Contributions - [ ] Used `/prompt-analyze` to review contribution - [ ] Addressed all feedback from `prompt-builder` review - [ ] Verified contribution follows common standards and type-specific requirements ### Required Automated Checks The following validation commands must pass before merging: - [x] Markdown linting: `npm run lint:md` - [x] Spell checking: `npm run spell-check` - [x] Frontmatter validation: `npm run lint:frontmatter` - [ ] Link validation: `npm run lint:md-links` - [x] PowerShell analysis: `npm run lint:ps` ## Security Considerations - [x] This PR does not contain any sensitive or NDA information - [ ] Any new dependencies have been reviewed for security issues - [x] Security-related scripts follow the principle of least privilege ## Additional Notes - Bash was intentionally excluded from the Supported Languages table; it may be added in a future iteration - The 80% code coverage threshold applies to skill scripts only; the repository-wide informational baseline remains at 18% - No GHCP artifacts (instructions, prompts, agents, skills) were modified in this PR 🧪 - Generated by Copilot --------- Co-authored-by: Katrien De Graeve <katriendg@users.noreply.github.com>
1 parent bd7d512 commit 9263617

7 files changed

Lines changed: 150 additions & 4 deletions

File tree

.cspell.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
"learning",
7373
"MMDD",
7474
"pullrequest",
75+
"pyproject",
7576
"rhysd",
7677
"SARIF",
7778
"Segoe",

.github/ISSUE_TEMPLATE/skill-request.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,18 @@ body:
1717
validations:
1818
required: true
1919

20+
- type: dropdown
21+
id: programming-language
22+
attributes:
23+
label: Programming Language
24+
description: Primary language for skill logic. All skills require Bash (macOS/Linux) and PowerShell (Windows) scripts for cross-platform support.
25+
options:
26+
- PowerShell
27+
- Python
28+
- Other
29+
validations:
30+
required: true
31+
2032
- type: textarea
2133
id: purpose
2234
attributes:

CONTRIBUTING.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,8 @@ CI reports coverage at an 18% informational baseline; focus on meaningful covera
308308
| Naming | `*.Tests.ps1` suffix matching source script name |
309309
| Framework | Pester 5.x |
310310

311+
Skill scripts use a different test location. Skill tests are co-located inside each skill directory at `.github/skills/<skill-name>/tests/` rather than mirrored under `scripts/tests/`. See [Skills Guide](./docs/contributing/skills.md#unit-testing-requirements) for details.
312+
311313
Minimal example:
312314

313315
```powershell

docs/architecture/testing.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,4 +171,34 @@ Run a specific test file:
171171
Invoke-Pester -Path ./scripts/tests/linting/Invoke-PSScriptAnalyzer.Tests.ps1
172172
```
173173

174+
## Skills Testing
175+
176+
Skill scripts use a co-located test pattern instead of the mirror directory structure used by `scripts/`. Each skill contains its own `tests/` subdirectory:
177+
178+
```text
179+
.github/skills/<skill-name>/
180+
├── scripts/
181+
│ ├── convert.ps1
182+
│ └── convert.sh
183+
└── tests/
184+
└── convert.Tests.ps1
185+
```
186+
187+
### Coverage Integration
188+
189+
The Pester configuration at `scripts/tests/pester.config.ps1` resolves skill scripts from the repository root for code coverage analysis. When you include a skill `tests/` directory in an `Invoke-Pester -Path` argument or test run configuration, Pester discovers the skill test files through the `.Tests.ps1` naming convention.
190+
191+
Coverage path resolution for skills uses the repository root rather than `$scriptRoot` (which points to `scripts/`):
192+
193+
```powershell
194+
$repoRoot = Split-Path $scriptRoot -Parent
195+
$skillScripts = Get-ChildItem -Path (Join-Path $repoRoot '.github/skills') `
196+
-Include '*.ps1', '*.psm1' -Recurse -File -ErrorAction SilentlyContinue |
197+
Where-Object { $_.FullName -notmatch '\.Tests\.ps1$' }
198+
```
199+
200+
### Packaging Exclusion
201+
202+
Co-located `tests/` directories are excluded from the VSIX extension package by `Package-Extension.ps1`. After copying a skill directory, the packaging script removes any `tests/` subdirectories from the destination.
203+
174204
🤖 *Crafted with precision by ✨Copilot following brilliant human instruction, then carefully refined by our team of discerning human reviewers.*

docs/contributing/skills.md

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ The following skill types will likely be **rejected**:
4343
* **Duplicate Skills**: Skills that replicate functionality of existing tools or skills
4444
* **Single-Platform Skills**: Skills that only work on one operating system
4545
* **Undocumented Utilities**: Scripts without comprehensive SKILL.md documentation
46+
* **Untested Skills**: Skills that lack unit tests or fail to achieve 80% code coverage
4647

4748
## File Structure Requirements
4849

@@ -53,10 +54,13 @@ All skill files **MUST** be placed in:
5354
```text
5455
.github/skills/<skill-name>/
5556
├── SKILL.md # Main skill definition (required)
56-
├── convert.sh # Bash script (required for cross-platform)
57-
├── convert.ps1 # PowerShell script (required for cross-platform)
58-
└── examples/
59-
└── README.md # Usage examples (recommended)
57+
├── scripts/
58+
│ ├── convert.ps1 # PowerShell script (required for cross-platform)
59+
│ └── convert.sh # Bash script (required for cross-platform)
60+
├── examples/
61+
│ └── README.md # Usage examples (recommended)
62+
└── tests/
63+
└── convert.Tests.ps1 # Pester unit tests (required for PowerShell)
6064
```
6165

6266
### Naming Convention
@@ -257,6 +261,69 @@ PowerShell scripts **MUST**:
257261
* Check for required dependencies
258262
* Use proper error handling
259263

264+
## Unit Testing Requirements
265+
266+
All skill scripts MUST include unit tests that achieve a minimum of 80% code coverage. Tests are co-located inside the skill directory to keep each skill self-contained.
267+
268+
### Test File Location
269+
270+
Place test files in a `tests/` subdirectory within the skill directory:
271+
272+
```text
273+
.github/skills/<skill-name>/
274+
└── tests/
275+
└── <script-name>.Tests.ps1
276+
```
277+
278+
### PowerShell Tests
279+
280+
PowerShell skill scripts require Pester 5.x tests:
281+
282+
* Use `.Tests.ps1` suffix matching the source script name
283+
* Follow the same conventions as `scripts/tests/` (see [Testing Architecture](../architecture/testing.md))
284+
* Pester configuration is defined at `scripts/tests/pester.config.ps1`; co-located skill tests run when their `tests/` directories are included in the Pester run paths (for example via CI or explicit test invocation)
285+
286+
Minimal example:
287+
288+
```powershell
289+
Describe 'Convert-VideoToGif' {
290+
It 'Validates input file exists' {
291+
{ ./convert.ps1 -InputPath 'nonexistent.mp4' } | Should -Throw
292+
}
293+
}
294+
```
295+
296+
### Python Tests
297+
298+
Python skill scripts require pytest:
299+
300+
* Use `test_<script_name>.py` naming convention
301+
* Place tests in the `tests/` subdirectory alongside PowerShell tests
302+
* Configure pytest and ruff in a `pyproject.toml` at the skill root
303+
304+
### Packaging Note
305+
306+
Co-located `tests/` directories are automatically excluded from the VSIX extension package. No additional contributor action is needed.
307+
308+
## Supported Languages
309+
310+
Skills may include scripts in any of these supported languages. Each language has specific tooling and CI expectations.
311+
312+
| Language | Script Extension | Test Framework | Linter / Analyzer | CI Coverage |
313+
|------------|------------------|----------------|---------------------------------------------|--------------------|
314+
| Bash | `.sh` | N/A | shellcheck | Lint only |
315+
| PowerShell | `.ps1` | Pester 5.x | PSScriptAnalyzer | Full (lint + test) |
316+
| Python | `.py` | pytest | ruff (line-length=88, target-version=py311) | Planned |
317+
318+
### Requesting New Language Support
319+
320+
To request support for a new programming language:
321+
322+
1. Open a [Skill Request](https://github.com/microsoft/hve-core/issues/new?template=skill-request.yml) issue
323+
2. Select the desired language in the Programming Language dropdown (choose "Other" if unlisted)
324+
3. Describe the tooling requirements: test framework, linter, CI integration needs
325+
4. A maintainer will evaluate feasibility and update this table when support is added
326+
260327
## Examples Directory
261328

262329
The `examples/` subdirectory **SHOULD** include:
@@ -291,6 +358,12 @@ Before submitting your skill, verify:
291358
* [ ] Both scripts implement equivalent functionality
292359
* [ ] Help and usage documentation included
293360

361+
### Testing
362+
363+
* [ ] Unit tests present in `tests/` subdirectory
364+
* [ ] PowerShell tests use `.Tests.ps1` naming convention
365+
* [ ] Tests pass locally via `npm run test:ps`
366+
294367
### Documentation
295368

296369
* [ ] All required SKILL.md sections present
@@ -307,6 +380,7 @@ Run these commands before submission:
307380
npm run lint:frontmatter # Validate SKILL.md frontmatter
308381
npm run psscriptanalyzer # Validate PowerShell scripts
309382
npm run lint # Validate markdown formatting
383+
npm run test:ps # Run PowerShell unit tests
310384
```
311385

312386
All checks **MUST** pass before merge.

scripts/extension/Package-Extension.ps1

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,10 @@ function Copy-CollectionArtifacts {
563563
$destDir = Split-Path $destPath -Parent
564564
New-Item -Path $destDir -ItemType Directory -Force | Out-Null
565565
Copy-Item -Path $srcPath -Destination $destPath -Recurse -Force
566+
567+
# Remove co-located test directories from packaged skills
568+
Get-ChildItem -Path $destPath -Directory -Filter 'tests' -Recurse -ErrorAction SilentlyContinue |
569+
Remove-Item -Recurse -Force
566570
}
567571
}
568572
}

scripts/tests/pester.config.ps1

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,29 @@ if ($CodeCoverage.IsPresent) {
5858
$_.FullName -notmatch '\.Tests\.ps1$'
5959
} | Select-Object -ExpandProperty FullName
6060

61+
# Resolve skill script coverage paths from repo root
62+
$repoRoot = Split-Path $scriptRoot -Parent
63+
$skillsPath = Join-Path $repoRoot '.github/skills'
64+
if (Test-Path $skillsPath) {
65+
$skillCoveragePaths = Get-ChildItem -Path $skillsPath -Directory -ErrorAction SilentlyContinue | ForEach-Object {
66+
$skillRoot = $_.FullName
67+
$skillScripts = Join-Path $skillRoot 'scripts'
68+
$paths = @()
69+
70+
$paths += Get-ChildItem -Path $skillRoot -Include '*.ps1', '*.psm1' -File -ErrorAction SilentlyContinue
71+
72+
if (Test-Path $skillScripts) {
73+
$paths += Get-ChildItem -Path $skillScripts -Include '*.ps1', '*.psm1' -Recurse -File -ErrorAction SilentlyContinue
74+
}
75+
76+
$paths
77+
} | Where-Object { $_.FullName -notmatch '\.Tests\.ps1$' } |
78+
Select-Object -ExpandProperty FullName
79+
if ($skillCoveragePaths) {
80+
$coveragePaths = @($coveragePaths) + @($skillCoveragePaths)
81+
}
82+
}
83+
6184
if ($coveragePaths.Count -gt 0) {
6285
$configuration.CodeCoverage.Path = $coveragePaths
6386
}

0 commit comments

Comments
 (0)