-
Notifications
You must be signed in to change notification settings - Fork 152
Add PR integration test skill #482
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
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| --- | ||
| name: pr-integration-test | ||
| description: 'Design, implement, and validate Intelligent Terminal integration tests for a target pull request or regression. Use when asked to add PR integration tests, convert a bug fix into E2E coverage, prove existing behavior still works, map tests to the release checklist, or verify E2E reports mark checklist cases complete.' | ||
| --- | ||
|
|
||
| # PR Integration Test | ||
|
|
||
| Turn a target PR into durable, behavior-focused integration coverage that proves | ||
| the fixed path, protects existing behavior, and updates the generated release | ||
| checklist. | ||
|
|
||
| ## When to Use This Skill | ||
|
|
||
| - Convert an open or merged PR into cross-component regression coverage. | ||
| - Extend `doc/release-check-list.md` and make report scripts check the new rows. | ||
| - Audit whether a PR's existing tests cover the user-visible behavior rather | ||
| than only its implementation details. | ||
|
|
||
| ## Prerequisites | ||
|
|
||
| - Read `test/e2e/README.md` and reuse the ItE2E framework instead of creating a | ||
| parallel harness. | ||
| - Run `pwsh -File test/e2e/bootstrap.ps1 -Check` before live E2E validation. | ||
|
|
||
| ## Workflow | ||
|
|
||
| Follow [workflow.md](./references/workflow.md). Track the phases with a TODO | ||
| list. | ||
|
|
||
| `analyze PR -> reconstruct behavior -> audit coverage -> design matrix -> write ItE2E -> wire checklist -> validate -> deliver` | ||
|
|
||
| ## Test Design Standard | ||
|
|
||
| For every proposed case, record: | ||
|
|
||
| | Field | Required answer | | ||
| |-------|-----------------| | ||
| | Contract | What user-visible behavior must remain true? | | ||
| | Trigger | What exact action or input exercises it? | | ||
| | Boundary | Which real component handoff does this test add beyond unit tests? | | ||
| | Oracle | What deterministic observable proves success or suppression? | | ||
| | Negative control | What similar input must not trigger the behavior? | | ||
| | Existing protection | Which existing tests protect old behavior and must still run? | | ||
| | Checklist title | Which exact bold release-checklist title contains the Pester test name? | | ||
|
|
||
| ## Oracle Priority | ||
|
|
||
| Prefer the earliest deterministic product-owned signal: | ||
|
|
||
| 1. Protocol/event stream | ||
| 2. Persisted or queryable application state | ||
| 3. Structured diagnostic log | ||
| 4. Rendered terminal or UI state | ||
| 5. LLM-generated text | ||
|
|
||
| Use model output only when it is the behavior under test; never use it to prove | ||
| routing, triggers, suppression, or idempotency. | ||
|
|
||
| ## Release Checklist Contract | ||
|
|
||
| - Add one unchecked `[E2E]` checklist item per independently releasable behavior. | ||
| - Give each item a concise bold title that appears verbatim in the matching | ||
| Pester full name (`Describe.Context.It`). | ||
| - Prefer exact-title matching. Add `test/e2e/release-coverage-map.psd1` entries | ||
| only when an exact test name would be misleading or one case intentionally | ||
| covers multiple checklist items. | ||
| - Assign stable IDs and verify `[x]` output through the full and incremental | ||
| report paths in [workflow.md](./references/workflow.md). | ||
|
|
||
| ## Completion Gate | ||
|
|
||
| Do not call the work complete until all of these are true: | ||
|
|
||
| - Pre-fix evidence identifies the regression, and the fixed path crosses the | ||
| real integration boundary. | ||
| - Relevant false positives, replay risks, and existing behavior are covered. | ||
| - The correct build passes related suites and marks new checklist IDs `[x]`; | ||
| every skip is explained. | ||
|
|
||
| ## Gotchas | ||
|
|
||
| - **Do not test only the implementation detail named in the PR.** Reconstruct | ||
| the end-to-end user path and assert the observable contract. | ||
| - **Do not duplicate a unit test at E2E level.** Add the missing process, | ||
| protocol, persistence, packaging, or UI boundary. | ||
| - **Do not use a successful lower-layer event as proof of the final feature.** | ||
| When the contract is downstream, assert both the trigger and its downstream | ||
| effect. | ||
| - **Do not turn product failures into skips.** Skip only when an external | ||
| prerequisite is genuinely unavailable. A connected product that behaves | ||
| incorrectly must fail. | ||
|
|
||
| ## References | ||
|
|
||
| - [Detailed PR-to-integration-test workflow](./references/workflow.md) | ||
| - [ItE2E framework](../../../test/e2e/README.md) | ||
| - [Release checklist](../../../doc/release-check-list.md) |
234 changes: 234 additions & 0 deletions
234
.github/skills/pr-integration-test/references/workflow.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,234 @@ | ||
| # PR-to-Integration-Test Workflow | ||
|
|
||
| Use this procedure for a target Intelligent Terminal PR. Adapt the exact test | ||
| suite and build commands to the files changed by that PR. | ||
|
|
||
| ## 1. Resolve the Target and Branch Strategy | ||
|
|
||
| Inspect the PR before reading implementation files: | ||
|
|
||
| ```powershell | ||
| $prNumber = 482 | ||
| gh pr view $prNumber --repo microsoft/intelligent-terminal ` | ||
| --json number,title,body,state,mergedAt,mergeCommit,baseRefName,headRefName,commits,files,closingIssuesReferences,reviews,url | ||
| gh pr diff $prNumber --repo microsoft/intelligent-terminal | ||
| ``` | ||
|
|
||
| Read linked issues and relevant review threads. Record: | ||
|
|
||
| - The pre-fix user symptom and exact reproduction. | ||
| - The intended behavior and explicitly accepted limitations. | ||
| - The changed components and every boundary crossed at runtime. | ||
| - Whether the PR is open, merged, or superseded. | ||
|
|
||
| Choose the branch deliberately: | ||
|
|
||
| - **Merged PR / follow-up test PR:** update the PR's base branch with | ||
| `git pull --ff-only`, delete the merged feature branch only after verifying | ||
| merge state, then create a new test branch from the merged commit. | ||
| - **Open PR and tests belong in it:** use the PR head only when the user expects | ||
| commits on that branch and it is safe to push there. | ||
| - **Open PR but independent validation is requested:** create a branch from the | ||
| PR head and clearly state that the test branch depends on the unmerged PR. | ||
|
|
||
| Verify merge state and a clean worktree before deleting a branch. Squash-merged | ||
| branches may require `git branch -D` because Git cannot infer ancestry. | ||
|
|
||
| ## 2. Reconstruct the Behavioral Contract | ||
|
|
||
| Describe the full path as components and observable handoffs: | ||
|
|
||
| ```text | ||
| user trigger | ||
| -> producer/component A | ||
| -> protocol or process boundary | ||
| -> consumer/component B | ||
| -> user-visible effect | ||
| ``` | ||
|
|
||
| For a regression, distinguish: | ||
|
|
||
| - What the producer emitted before the fix. | ||
| - What downstream code interpreted. | ||
| - Why existing tests did not catch the gap. | ||
| - Which observable separates the regression from a legitimate success case. | ||
|
|
||
| ## 3. Audit Existing Coverage and Harness Primitives | ||
|
|
||
| Search before adding helpers: | ||
|
|
||
| ```powershell | ||
| $pattern = 'feature|event|setting|command' | ||
| git grep -n -E $pattern -- test/e2e tools/wta/src src/cascadia | ||
| ``` | ||
|
|
||
| Read: | ||
|
|
||
| - Related `test/e2e/tests/Feature.*.Tests.ps1` suites. | ||
| - Relevant functions under `test/e2e/ItE2E/Public/`. | ||
| - Unit tests around classification, state transitions, and parsing. | ||
| - Matching items in `doc/release-check-list.md`. | ||
| - `test/e2e/release-coverage-map.psd1`. | ||
| - `test/e2e/release-exclude.psd1`, so a new title is not silently omitted from | ||
| the generated report. | ||
|
|
||
| Classify current coverage: | ||
|
|
||
| | Layer | What it should prove | | ||
| |-------|----------------------| | ||
| | Unit | Local decisions, parsing, reducer/state-machine branches | | ||
| | Component | Generated scripts, serialization, API adapters | | ||
| | Integration/E2E | Real process/protocol/package/UI wiring | | ||
| | Release checklist | Which user-facing behaviors count as signed off | | ||
|
|
||
| Add a shared helper only when multiple suites need it or it provides a more | ||
| precise oracle. | ||
|
|
||
| ## 4. Build the Behavior Matrix | ||
|
|
||
| Start with the regression, then add only risk-driven controls: | ||
|
|
||
| 1. **Regression positive:** the exact old failure now reaches the intended final | ||
| effect. | ||
| 2. **Ordinary baseline:** the preexisting successful or failure path still | ||
| works. | ||
| 3. **False-positive control:** a similar but legitimate case remains ignored. | ||
| 4. **Replay/idempotency:** redraw, retry, duplicate event, or repeated command | ||
| does not double-submit or reuse stale state. | ||
| 5. **Lifecycle/routing:** hidden panes, split panes, moved tabs, reconnects, or | ||
| process restarts only when the PR touches those risks. | ||
| 6. **Compatibility:** alternate shell, agent, policy, or language mode only when | ||
| the changed code is shared with it. | ||
|
|
||
| Each case must correspond to a plausible failure mode introduced or exposed by | ||
| the target PR. | ||
|
|
||
| For every case, identify both the immediate trigger and the downstream effect. | ||
| For example, proving a failure event exists is insufficient when the user | ||
| contract is that Autofix receives it; assert the event and the Autofix request. | ||
|
|
||
| ## 5. Implement Deterministic ItE2E Tests | ||
|
|
||
| Follow existing suite structure: | ||
|
|
||
| ```powershell | ||
| #Requires -Modules @{ ModuleName='Pester'; ModuleVersion='5.0.0' } | ||
|
|
||
| BeforeDiscovery { | ||
| $package = Get-AppxPackage | Where-Object Name -like '*IntelligentTerminal*' | ||
| $script:Ready = $null -ne $package | ||
| } | ||
|
|
||
| Describe 'Feature: <behavior>' -Tag 'Feature' -Skip:(-not $script:Ready) { | ||
| BeforeAll { | ||
| Import-Module (Join-Path $PSScriptRoot '..\ItE2E\ItE2E.psd1') -Force | ||
| $script:app = Start-Terminal -Package (Get-ItTestPackage) -PassFre $true | ||
| } | ||
| AfterAll { if ($script:app) { Stop-Terminal -App $script:app } } | ||
|
|
||
| It '<exact release-checklist title>' { | ||
| # Start observers before the action, scope the oracle, and assert the | ||
| # real downstream contract. | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| Implementation rules: | ||
|
|
||
| - Start listeners before the action; scope predicates by stable IDs and poll for | ||
| positive outcomes. | ||
| - For a negative case, first prove the action completed, then use a bounded | ||
| observation window. | ||
| - Use unique inputs when the product deduplicates, isolate state between cases, | ||
| and clean up in `finally`/`AfterAll`. | ||
| - Keep model-semantic assertions separate from deterministic pipeline checks. | ||
|
|
||
| ## 6. Wire the Release Checklist | ||
|
|
||
| Add unchecked E2E items near the related feature section: | ||
|
|
||
| ```markdown | ||
| - [ ] `[new]` `[E2E]` **Exact behavior title:** User-visible contract. _(#issue; E2E: `Feature.Suite`.)_ | ||
| ``` | ||
|
|
||
| Use the same `Exact behavior title` in the Pester `It` name. Then assign IDs: | ||
|
|
||
| ```powershell | ||
| pwsh -NoProfile -File test/e2e/Set-ChecklistIds.ps1 | ||
| ``` | ||
|
|
||
| Never reuse, insert, or renumber IDs manually. If exact-title matching is not | ||
| appropriate, add a narrow regex to `test/e2e/release-coverage-map.psd1` and | ||
| explain why it cannot over-credit another behavior. | ||
|
|
||
| Update the suite table in `test/e2e/README.md` when adding a new feature file. | ||
|
|
||
| ## 7. Validate Tests and Report Mapping | ||
|
|
||
| Verify prerequisites: | ||
|
|
||
| ```powershell | ||
| pwsh -NoProfile -File test/e2e/bootstrap.ps1 -Check | ||
| ``` | ||
|
|
||
| Run the new suite through the report driver: | ||
|
|
||
| ```powershell | ||
| $suite = 'test/e2e/tests/Feature.AutofixParser.Tests.ps1' | ||
| pwsh -NoProfile -File test/e2e/Invoke-ItE2EReport.ps1 ` | ||
| -Path $suite ` | ||
| -UpdateReport | ||
| ``` | ||
|
|
||
| Confirm: | ||
|
|
||
| - No new case failed or skipped unexpectedly. | ||
| - Every new checklist ID appears as `- [x]` in | ||
| `test/e2e/artifacts/release-report.md`. | ||
| - A failure would produce `AUTOMATION FAILED`, not a false pass. | ||
|
|
||
| `-UpdateReport` incrementally overlays matched results when a prior report | ||
| exists and falls back to a fresh report otherwise. To validate the underlying | ||
| incremental script explicitly or use a custom output directory, run: | ||
|
|
||
| ```powershell | ||
| $report = 'test/e2e/artifacts/release-report.md' | ||
| $results = 'test/e2e/artifacts/results.xml' | ||
| $updated = 'test/e2e/artifacts/release-report-updated.md' | ||
| pwsh -NoProfile -File test/e2e/Update-ReleaseReport.ps1 ` | ||
| -Report $report ` | ||
| -ResultsXml $results ` | ||
| -OutFile $updated | ||
| ``` | ||
|
|
||
| Verify only matched items changed and every new ID is `[x]`. A skipped-only | ||
| result must leave an existing checkbox unchanged. | ||
|
|
||
| Run related existing suites in the same Pester invocation when practical. At | ||
| minimum include: | ||
|
|
||
| - The suite that previously covered the ordinary path. | ||
| - The lower-layer suite that produces the new trigger. | ||
| - Routing/lifecycle suites affected by shared state. | ||
|
|
||
| Escalate to the broader Feature suite only when targeted runs expose shared | ||
| regressions or the PR changes common harness/product infrastructure. | ||
|
|
||
| ## 8. Prove the Correct Build Ran | ||
|
|
||
| Build and deploy the changed area. For WTA changes, build the explicit target | ||
| matching the package architecture before the C++ package, deploy it, select it | ||
| with `ITE2E_PACKAGE` or `-Package Dev`, and verify a runtime version, path, log, | ||
| or changed observable. Compilation alone does not prove the deployed package | ||
| contains the new `wta.exe` or generated shell integration. | ||
|
|
||
| ## 9. Deliver the Test PR | ||
|
|
||
| Before committing, run `git -c core.whitespace=cr-at-eol diff --check`. | ||
|
|
||
| The PR description must include: | ||
|
|
||
| - Target PR/issue and the integration boundary added. | ||
| - Cases and checklist IDs. | ||
| - Targeted/regression totals and skip reasons. | ||
| - Package/build tested. | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.