Delete unit tests - #2294
Merged
JasonYeMSFT (JasonYeMSFT) merged 2 commits intoMay 18, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Removes the Jest-based unit test layer for skills/utilities and updates the test runner and documentation to focus on trigger + integration testing.
Changes:
- Deleted many
unit.test.tssuites (skills + utils) and removed the “unit tests only” runner mode. - Removed quality/coverage reporting scripts from the
testspackage scripts. - Updated docs/templates/checklists to no longer reference unit tests.
Show a summary per file
| File | Description |
|---|---|
| tests/utils/tests/unit.test.ts | Removes utility unit coverage (including redactSecrets). |
| tests/scripts/run-tests.js | Removes unit test mode from the test runner. |
| tests/scripts/generate-quality-report.js | Deletes the quality report generator script. |
| tests/package.json | Removes coverage:grid and quality-report npm scripts. |
| tests/microsoft-foundry/unit.test.ts | Deletes microsoft-foundry unit tests. |
| tests/microsoft-foundry/resource/create/unit.test.ts | Deletes sub-skill unit tests. |
| tests/microsoft-foundry/models/deploy/deploy-model/unit.test.ts | Deletes router unit tests. |
| tests/microsoft-foundry/models/deploy/deploy-model-optimal-region/unit.test.ts | Deletes unit tests. |
| tests/microsoft-foundry/models/deploy/customize-deployment/unit.test.ts | Deletes unit tests. |
| tests/microsoft-foundry/models/deploy/capacity/unit.test.ts | Deletes unit tests. |
| tests/microsoft-foundry/foundry-agent/troubleshoot/unit.test.ts | Deletes unit tests. |
| tests/microsoft-foundry/foundry-agent/trace/unit.test.ts | Deletes unit tests. |
| tests/microsoft-foundry/foundry-agent/observe/unit.test.ts | Deletes unit tests. |
| tests/microsoft-foundry/foundry-agent/invoke/unit.test.ts | Deletes unit tests. |
| tests/microsoft-foundry/foundry-agent/eval-datasets/unit.test.ts | Deletes unit tests. |
| tests/microsoft-foundry/foundry-agent/deploy/unit.test.ts | Deletes unit tests. |
| tests/microsoft-foundry/foundry-agent/create/unit.test.ts | Deletes unit tests. |
| tests/mcp-tool-references/unit.test.ts | Removes unit validation for MCP tool references in markdown. |
| tests/entra-app-registration/unit.test.ts | Deletes skill unit tests. |
| tests/entra-agent-id/unit.test.ts | Deletes skill unit tests. |
| tests/azure-validate/unit.test.ts | Deletes skill unit tests. |
| tests/azure-upgrade/unit.test.ts | Deletes skill unit tests (including pytest wrapper checks). |
| tests/azure-resource-visualizer/unit.test.ts | Deletes skill unit tests. |
| tests/azure-resource-lookup/unit.test.ts | Deletes skill unit tests. |
| tests/azure-reliability/unit.test.ts | Deletes skill unit tests. |
| tests/azure-rbac/unit.test.ts | Deletes skill unit tests. |
| tests/azure-quotas/unit.test.ts | Deletes skill unit tests. |
| tests/azure-prepare/unit.test.ts | Deletes skill unit tests. |
| tests/azure-messaging/unit.test.ts | Deletes skill unit tests. |
| tests/azure-kubernetes/unit.test.ts | Deletes skill unit tests. |
| tests/azure-kubernetes/azure-kubernetes-automatic-readiness/unit.test.ts | Deletes sub-skill unit tests. |
| tests/azure-hosted-copilot-sdk/unit.test.ts | Deletes skill unit tests. |
| tests/azure-enterprise-infra-planner/unit.test.ts | Deletes skill unit tests. |
| tests/azure-diagnostics/unit.test.ts | Deletes skill unit tests. |
| tests/azure-deploy/unit.test.ts | Deletes skill unit tests. |
| tests/azure-cost/unit.test.ts | Deletes skill unit tests. |
| tests/azure-compute/unit.test.ts | Deletes skill unit tests. |
| tests/azure-compliance/unit.test.ts | Deletes skill unit tests. |
| tests/azure-cloud-migrate/unit.test.ts | Deletes skill unit tests. |
| tests/azure-aigateway/unit.test.ts | Deletes skill unit tests. |
| tests/azure-ai/unit.test.ts | Deletes skill unit tests. |
| tests/appinsights-instrumentation/unit.test.ts | Deletes skill unit tests. |
| tests/airunway-aks-setup/unit.test.ts | Deletes skill unit tests. |
| tests/_template/unit.test.ts | Removes the unit test template file. |
| tests/_template/README.md | Removes unit test documentation from template docs. |
| tests/README.md | Updates documentation to remove discussion of unit tests (but leaves some stale wording). |
| tests/AGENTS.md | Updates contributor instructions to remove unit test steps (but leaves some stale wording). |
| docs/spec/azure-hosted-copilot-sdk.md | Removes references to unit test template and unit tests in the spec doc. |
| .github/skills/skill-reviewer/references/review-checklist.md | Removes unit test requirement from checklist. |
| .github/copilot-instructions.md | Removes test:unit command reference. |
| .github/agents/SkillCreator.agent.md | Updates skill creation requirements to omit unit tests. |
Copilot's findings
Comments suppressed due to low confidence (3)
tests/package.json:1
- The PR description focuses on removing 'trivial keyword' unit tests for skills, but this change also removes other tooling entry points (
coverage:grid,quality-report) and deletes the underlying quality report generator script. If that broader scope is intended, it should be reflected in the PR description (and any contributor docs that still reference these commands should be updated accordingly).
tests/scripts/run-tests.js:1 - By removing the
unitmode, there is no longer a built-in way to run a fast local subset that excludes integration tests (which are often slower and may require more environment setup). Consider adding a replacement mode (e.g.,fastornon-integration) that explicitly filters out integration tests, so contributors and CI jobs can choose the appropriate test scope without manual Jest flags.
tests/scripts/run-tests.js:1 - By removing the
unitmode, there is no longer a built-in way to run a fast local subset that excludes integration tests (which are often slower and may require more environment setup). Consider adding a replacement mode (e.g.,fastornon-integration) that explicitly filters out integration tests, so contributors and CI jobs can choose the appropriate test scope without manual Jest flags.
- Files reviewed: 51/51 changed files
- Comments generated: 2
Tom Meschter (tmeschter)
previously approved these changes
May 18, 2026
Tom Meschter (tmeschter)
approved these changes
May 18, 2026
This was referenced May 21, 2026
Sai Koumudi Kaluvakolanu (saikoumudi)
pushed a commit
that referenced
this pull request
May 26, 2026
* Add azure-reliability skill with checklist-based assessment
Adds a new azure-reliability skill that assesses Azure PaaS apps (Functions, Container Apps, App Service) for zone redundancy, storage replication, multi-region, and health probes. Reports findings as an enabled/disabled checklist (no numeric scoring) and supports both live (CLI) and IaC (Bicep/Terraform) remediation paths.
* azure-reliability: address test-feedback (AVM, FC1, JSON output, default SKU, deploy order)
- Note AVM module param naming differs from raw Bicep (skuName vs sku.name); detect with Select-String and patch the actual param in use.
- Annotate FC1/Consumption health probe as 'code-only fix' in the checklist and risk table; do not patch healthCheckPath in IaC for these plans.
- Switch all 'az graph query' examples to '--query data[] -o json' (table output only shows summary cols).
- Handle no-SKU storage case: ARM/AVM defaults to Standard_GRS; add explicit sku/skuName instead of find-and-replace.
- Recommend splitting deploys: safe patches first, then storage migration, then storage SKU patch (a failed redundancy update can fail the whole deploy).
* azure-reliability: require user consent before adding HTTP health trigger on FC1/Consumption
Adding a /api/health endpoint on Flex Consumption / Consumption Function Apps means modifying app source code (new HTTP trigger), not a Bicep/Terraform patch. Add an explicit STOP gate in configure-health-probes.md, the SKILL.md risk table, and both IaC patching references so the agent must ask the user before touching code, and respects 'no' by leaving everything unchanged.
* azure-reliability: do quick wins first, ask before storage migration and multi-region
Workflow now sequences remediation as: easy ZR + health probes first (CLI or 'Deploy 1' for IaC), then explicitly ASK the user before kicking off the slow storage migration ('to be fully ZR you also need ZRS storage - want to do it?'). After the storage step (or skip), re-assess, then a new Configuration Workflow Step 3 asks about multi-region failover and waits for yes/no/later before generating any Front Door IaC. Phase 3 UX note now defers to Step 3 instead of duplicating the offer.
* azure-reliability: pivot assessment table to feature-rows for clarity
Replace per-resource checklist (with mostly n/a cells and mixed symbols) with a feature-pivoted table: 4 rows (Zone redundancy compute, Zone-redundant storage, Health probes, Multi-region failover), each with a single status (🟢 ON / 🟡 PARTIAL / 🔴 OFF or storage SKU) and a bullet list of relevant resources with inline reasons. Drops n/a noise; reasons sit on the resource line where users can see them. Re-Assess uses the same format with 'now ON' / 'still off' annotations. Step 3 prompt text aligned to use 🟢 instead of ✅.
* azure-reliability: skill executes deploys itself instead of telling user to run them
Path B Deploy 1, Storage migration, Deploy 2, and Multi-region all now run the deploy commands (azd up / az deployment / terraform apply) directly after a single yes/no confirmation, then continue to re-assess. Updates Skill Boundaries and Integration tables: 'Deploy IaC for reliability changes: Yes' (was 'No, hand off to azure-deploy'). Bicep and Terraform patching references updated to summarize the deploy plan and ask 'Ready for Deploy 1?' instead of giving the user a list of commands to run.
* azure-reliability: remove Integration with Other Skills section
* azure-reliability: dedup overlapping content + fix literal escape bugs
Removed duplications:
- 'When to Use This Skill' section (duplicate of Skill Activation Triggers)
- 'HARD STOPS' block (each stop already inline at the right step)
- Best Practices items that restated the workflow (kept only the 2 unique tips)
- Path B Step 2 'Deploy-order rule' callout (Steps 3-5 already detail the flow)
- Phase 3 UX note about multi-region (reduced to a one-liner pointing to Step 3)
- Skill Boundaries 'IMPORTANT' header (duplicated description / Quick Reference)
Also:
- Quick Reference now says 'Reliability assessment table' instead of 'Reliability Checklist' for consistency with Phase 3 terminology.
- Storage SKU row in Step 2 risk table now points to the two-deploy flow inline.
- Fixed literal \\u escape sequences in SKILL.md, iac-patching-bicep.md, iac-patching-terraform.md that leaked from a previous edit (\\u2014 -> em-dash, \\u2192 -> arrow, \\u26a0\\ufe0f -> warning, \\u2705 -> checkmark, etc).
* azure-reliability: fix known issues in services/{container-apps,app-service,functions} before team handoff
Container Apps:
- Standardize 'az graph query' to use --query 'data[]' -o json (table mode hides projected fields)
- Fix Configure: Health Probes example (was mixing --set-env-vars + --yaml -<<EOF heredoc, which conflicts and isn't pwsh-portable); use a probes.yaml file instead
- Add AVM modules note pointing to br/public:avm/res/app/managed-environment and avm/res/app/container-app param naming
- Add STOP gate before adding /health route to container code (consent required, mirrors Functions FC1 pattern)
- Add Reporting section showing how each Container Apps resource maps to feature-pivoted assessment rows
App Service:
- Standardize 'az graph query' to use --query 'data[]' -o json
- Add ARR affinity / clientAffinityEnabled query (sticky sessions break ZR + multi-region)
- Add new 'Configure: Disable Client Affinity (ARR Affinity)' section above slots; multi-region note now references it
- Add AVM modules note pointing to br/public:avm/res/web/serverfarm and avm/res/web/site param naming
- Add Reporting section, including PARTIAL state for multi-region with affinity still enabled
Functions:
- Add Reporting section so Functions matches the new convention
Deferred for the App Service / Container Apps teams to verify against current Microsoft docs:
- P1v3 minimum capacity for ZR (table currently says 3; docs may now say 2)
- Standard tier health check support (table claims yes; verify)
- Auto-Heal, backup/restore, VNet integration HA notes (out of scope for this pass)
* azure-reliability: pivot to per-service ownership for service-specific content
Make each services/<svc>/reliability.md the single source of truth for that service's plan/SKU rules, assessment queries, CLI commands, IaC patches (Bicep + Terraform + AVM), and reporting hints. Strip duplicated content from shared references so they only contain platform-level mechanics:
- zone-redundancy-checks.md: keep platform overview + cross-service all-in-one query + AZ regions list. Drop per-service queries and remediation (already in services/).
- configure-zone-redundancy.md: become a thin pointer to per-service refs + storage prerequisite + verification command.
- configure-health-probes.md: become a thin pointer to per-service refs + cross-service consent gate + best practices.
- iac-patching-bicep.md / iac-patching-terraform.md: keep framework (When to Use, Detection, AVM modules note, deploy plan) and the single cross-service Storage patch. Per-service patches now live in services/.
- health-probe-checks.md: keep Front Door / Traffic Manager / App Insights checks + best practices + Multi-region row reporting. Drop per-service queries.
- SKILL.md Phase 2 reworded as 'platform discovery + per-service deep dive'. Path A and Path B Step 3 now point to per-service refs for compute commands/patches.
Net -889 lines (no content lost; the deleted lines were already duplicated in services/).
This sets up a clean 3-PR split: PR #1 = shared platform refs + services/functions/, PR #2 = services/app-service/, PR #3 = services/container-apps/.
* azure-reliability: restore Y1 upgrade path notes (Functions) and shared-env warning (Container Apps)
Two pieces of unique safety content from the deleted shared files were not yet captured in the per-service files. Adding them back:
- Functions: 'Consumption (Y1) - upgrade path required' subsection with Flex vs Premium tradeoffs and cost warning (was in deleted configure-zone-redundancy.md).
- Container Apps: explicit STOP before deleting old environment, with the 'az containerapp list --environment' check command (was in deleted configure-zone-redundancy.md).
* Scope to Functions for initial PR; App Service + Container Apps land in companion PRs
* Add App Service reliability reference
* Apply suggestion from @apwestgarth
Co-authored-by: Andrew Westgarth <mail@hawaythelads.co.uk>
* Apply suggestion from @apwestgarth
Co-authored-by: Andrew Westgarth <mail@hawaythelads.co.uk>
* Apply suggestion from @apwestgarth
Co-authored-by: Andrew Westgarth <mail@hawaythelads.co.uk>
* Apply suggestion from @apwestgarth
Co-authored-by: Andrew Westgarth <mail@hawaythelads.co.uk>
* Apply suggestions from code review
Co-authored-by: Andrew Westgarth <mail@hawaythelads.co.uk>
* Update plugin/skills/azure-reliability/references/services/app-service/reliability.md
Co-authored-by: Andrew Westgarth <mail@hawaythelads.co.uk>
* Update plugin/skills/azure-reliability/references/services/app-service/reliability.md
Co-authored-by: Andrew Westgarth <mail@hawaythelads.co.uk>
* Update plugin/skills/azure-reliability/references/services/app-service/reliability.md
Co-authored-by: Andrew Westgarth <mail@hawaythelads.co.uk>
* Update plugin/skills/azure-reliability/references/services/app-service/reliability.md
Co-authored-by: Andrew Westgarth <mail@hawaythelads.co.uk>
* Enhance Azure Reliability skill for App Service support
Updated skill description to include Azure App Service and improved reliability assessment details. Adjusted supported services and clarified scope notes for Azure Functions and App Service.
* Revise health probe details in documentation
Updated health probe references for App Service
* Add Azure App Service reference for zone redundancy
* Clarify compute patches for App Service Plans
* Update Bicep patch references in documentation
Clarify service references and update patch descriptions for Azure App Service and Azure Functions.
* Update references for Azure App Service and Functions
* Update SKILL.md
Fixed error in description
* Update unit.test.ts
Update test to validate support for App Service
* Update test description for Azure Functions scope
Update test description and title to include App Service
* Fix spelling mistake in descripton - SKILL.md
Fixed spelling mistake - "Azure Azure App Service" to "Azure App Service"
* Update unit.test.ts
Fix casing of content expected in test as test converts to lowercase, expected text should also be lowercase.
* Update unit.test.ts
Fixed spelling mistake in expected description content
* Fix to ensure consistency and update unit test to correctly reflect changes
* Update snapshot descriptions and keywords for triggers
* Update test snapshot
* Update triggers.test.ts.snap
update the Jest snapshot to match the new azure-reliability skill metadata description.
* Update triggers.test.ts
Update test triggers
* Update triggers.test.ts.snap
Update Test Snapshot for azure-reliability tests
* Added App Service End to End Zone Redundancy Integration Test
* Apply suggestions from code review
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Andrew Westgarth <mail@hawaythelads.co.uk>
* closed fencing on code block for Auto Heal
* Deleted unit tests as no longer valuable due to #2294
* Update description in snapshot
---------
Co-authored-by: Andrew Westgarth <mail@hawaythelads.co.uk>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@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
The unit tests have been testing trival things such as whether the skill contains certain keywords or has certain patterns. These checks don't translate to the effectiveness of the skill and aren't really useful. Skill authors optimize their skills based on the actual behavior of agents. Maintaining another set of keyword matches are unnecessary work for our skills.
Checklist
cd tests && npm test)npm run test:skills:integration -- <skill>)USE FOR/DO NOT USE FOR/PREFER OVERclauses: confirmed no routing regressions for competing skillsRelated Issues