Skip to content

eval: migrate all remaining Foundry skill js tests to vally - #3068

Open
anchenyi wants to merge 5 commits into
mainfrom
anchenyi/clean-jtest
Open

eval: migrate all remaining Foundry skill js tests to vally#3068
anchenyi wants to merge 5 commits into
mainfrom
anchenyi/clean-jtest

Conversation

@anchenyi

@anchenyi anchenyi commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Description

Migrate all remaining Foundry skill js tests to vally.

Checklist

  • Tests pass locally (cd tests && npm test)
  • Title has one of the prefixes: fix:, feat:, feature:, chore:, misc:, test:, eval:
  • If modifying skill descriptions: verified routing correctness with integration tests (In tests/, npm run test:integration -- <skill> or npm run test:vally -- --skill <skill>)

Related Issues

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 removes legacy Jest test coverage for the top-level microsoft-foundry skill and the foundry-agent lifecycle area, relying on the newer per-scope test suites already present under tests/microsoft-foundry/**.

Changes:

  • Removed the top-level trigger test suite and its snapshot for microsoft-foundry.
  • Removed legacy integration tests for the top-level skill and foundry-agent.
  • Removed foundry-agent unit tests that locked down documentation/path invariants (direct-code + toolbox paths).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/microsoft-foundry/triggers.test.ts Deletes the legacy top-level trigger tests for the microsoft-foundry skill.
tests/microsoft-foundry/integration.test.ts Deletes the legacy top-level integration test suite for the skill.
tests/microsoft-foundry/foundry-agent/integration.test.ts Deletes the legacy foundry-agent integration tests.
tests/microsoft-foundry/foundry-agent/direct-code.unit.test.ts Deletes unit tests that asserted direct-code deployment documentation constraints.
tests/microsoft-foundry/foundry-agent/create/toolbox-paths.unit.test.ts Deletes unit tests validating toolbox reference paths and linked reference existence.
tests/microsoft-foundry/snapshots/triggers.test.ts.snap Deletes the snapshot file associated with the removed top-level trigger tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@anchenyi anchenyi changed the title eval: remove legacy JavaScript tests for the agent lifecycle scope in the Foundry skill eval: remove legacy JavaScript tests in the Foundry skill Aug 14, 2026
@anchenyi anchenyi changed the title eval: remove legacy JavaScript tests in the Foundry skill eval: remove JavaScript tests in the Foundry skill Aug 14, 2026
@anchenyi anchenyi changed the title eval: remove JavaScript tests in the Foundry skill eval: migrate all remaining Foundry skill js tests to vally Aug 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.

Suppressed comments (3)

evals/azure-skills/microsoft-foundry/eval.yaml:31

  • The PR removes Jest tests for microsoft-foundry:resource/create routing (and its snapshots), but this eval file doesn't appear to include any stimuli that cover Foundry resource creation prompts (e.g., creating an AIServices/multi-service resource, registering the provider). Add at least one routing stimulus for resource creation to preserve coverage.
  # ── ai-model-deployment-prompt ──
  # Jest: "invokes microsoft-foundry skill for AI model deployment prompt"
  # Assertions: softCheckSkill + isSkillInvoked (invocation rate ≥ 80%)
  - name: "AI model deployment from Foundry catalog"
    prompt: "How do I deploy an AI model from the Microsoft Foundry catalog?"
    tags:

evals/azure-skills/microsoft-foundry/eval.yaml:400

  • Several removed Jest unit tests were doing deterministic doc/contract validation (e.g., direct-code deploy guidance, toolbox sample path references). I couldn't find equivalent deterministic checks in this Vally suite (no file-matches/file-exists graders against the built skill markdown), so those regressions would no longer be caught. Consider migrating those unit assertions to Vally with file-matches graders, or keep a minimal Jest unit test for the static doc contracts.
  # ═══════════════════════════════════════════════════════════════════════════
  # Foundry Agent Create
  # ═══════════════════════════════════════════════════════════════════════════

  - name: "Create Python hosted Foundry agent"

evals/azure-skills/microsoft-foundry/eval.yaml:25

  • With 65 stimuli in this suite, defaults.runs: 5 implies ~325 total runs, which is likely to be slow/expensive and can increase flakiness (especially for response-quality regex graders). Consider setting the suite default to runs: 1 and overriding runs: 5 only for the handful of stimuli that actually need invocation-rate measurement.
stimuli:
  # ═══════════════════════════════════════════════════════════════════════════
  # Microsoft Foundry Routing
  # ═══════════════════════════════════════════════════════════════════════════

Comment thread evals/azure-skills/microsoft-foundry/eval.yaml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.

Suppressed comments (2)

evals/azure-skills/microsoft-foundry/eval.yaml:46

  • The migrated Vally evals cover many positive routing prompts, but there are no negative-routing checks asserting that unrelated prompts do not invoke microsoft-foundry (the deleted Jest trigger tests had explicit "Should NOT Trigger" cases). Add at least one negative stimulus using a skill-invocation grader with disallowed: [microsoft-foundry] to prevent regressions in routing precision.
      # Global: no_runtime_failure
      - type: output-not-matches
        config:
          pattern: "(?i)fatal error|unhandled exception|stack trace"

evals/azure-skills/microsoft-foundry/eval.yaml:565

  • In the "Fine-tuning guidance" stimulus, earlyTerminate stops as soon as the assistant mentions "training", but the graders currently also require a microsoft-foundry skill invocation. This can cause false failures if the model mentions training before routing/invoking the skill (the old Jest test allowed either skill invocation OR a fine-tuning/training mention). Consider dropping the mandatory skill-invocation grader for this response-quality check, or changing the termination condition to wait for skill routing.
      earlyTerminate: '[{"type":"assistant-message-match","contentPattern":"[Tt]raining"}]'
    graders:
      - type: skill-invocation
        config:
          required:

@JasonYeMSFT

JasonYeMSFT (JasonYeMSFT) commented Aug 17, 2026

Copy link
Copy Markdown
Member

anchenyi Since we added codeowner rules for each skill owner to also own the eval suites, you need someone from your team to approve the PR to merge it. The smake test failures aren't related to foundry so you can bypass it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants