.NET: Fix anthropic integration tests and skip reason#4211
Merged
westey-m merged 1 commit intomicrosoft:feature-xunit3-mtp-upgradefrom Feb 24, 2026
Merged
Conversation
fb73231
into
microsoft:feature-xunit3-mtp-upgrade
6 checks passed
Contributor
There was a problem hiding this comment.
Pull request overview
Updates several .NET integration tests to improve skip handling (notably for Anthropic) and to avoid configuration being loaded during type initialization when tests are skipped.
Changes:
- Replaced null-forgiving skip-reason usage (
!) with null-safe expressions in multiple integration test overrides. - Refactored Anthropic integration tests to avoid static configuration initialization and to use runtime skipping instead of
[Fact(Skip=...)]/ wrapper base classes. - Centralized Anthropic run-test skipping via the fixture’s
InitializeAsync()rather than per-test overrides.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| dotnet/tests/CopilotStudio.IntegrationTests/CopilotStudioRunTests.cs | Adjusts skip-reason argument passed to Assert.SkipWhen for manual-verification gating. |
| dotnet/tests/CopilotStudio.IntegrationTests/CopilotStudioRunStreamingTests.cs | Same skip-reason adjustment as non-streaming CopilotStudio tests. |
| dotnet/tests/AzureAIAgentsPersistent.IntegrationTests/AzureAIAgentsPersistentStructuredOutputRunTests.cs | Updates skip-reason argument passed to Assert.SkipWhen for flaky CI behavior. |
| dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicSkillsIntegrationTests.cs | Moves to runtime skipping and loads Anthropic config inside test methods instead of static init. |
| dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicChatCompletionRunTests.cs | Removes wrapper “skip-all” base class; derives directly from conformance RunTests<>. |
| dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicChatCompletionRunStreamingTests.cs | Removes wrapper “skip-all” streaming base class; derives directly from conformance RunStreamingTests<>. |
| dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicChatCompletionFixture.cs | Removes static config init; loads config lazily and adds skip behavior during fixture initialization. |
| dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicChatCompletionChatClientAgentRunTests.cs | Removes wrapper “skip-all” base class; derives directly from conformance ChatClientAgentRunTests<>. |
| dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicChatCompletionChatClientAgentRunStreamingTests.cs | Removes wrapper “skip-all” streaming base class; derives directly from conformance ChatClientAgentRunStreamingTests<>. |
Comment on lines
+21
to
22
| Assert.SkipWhen(ManualVerification is not null, ManualVerification ?? string.Empty); | ||
| return base.RunWithChatMessageReturnsExpectedResultAsync(); |
There was a problem hiding this comment.
Same pattern as the non-streaming tests: since ManualVerification is a constant non-null string, this SkipWhen condition is always true and ?? string.Empty is dead code. Consider using Assert.Skip(ManualVerification) or making the skip reason nullable if you want conditional skipping.
Comment on lines
10
to
16
| private const string SkipReason = "Fails intermittently on the build agent/CI"; | ||
|
|
||
| public override Task RunWithResponseFormatReturnsExpectedResultAsync() | ||
| { | ||
| Assert.SkipWhen(SkipReason is not null, SkipReason!); | ||
| Assert.SkipWhen(SkipReason is not null, SkipReason ?? string.Empty); | ||
| return base.RunWithResponseFormatReturnsExpectedResultAsync(); | ||
| } |
There was a problem hiding this comment.
SkipReason is a non-null const, so these overrides will always skip unconditionally and the SkipWhen(... is not null, ...) check is redundant. Consider simplifying to Assert.Skip(SkipReason) to make the intent explicit (or make the skip conditional on CI-only if you still want local runs).
| public sealed class AnthropicSkillsIntegrationTests | ||
| { | ||
| // All tests for Anthropic are intended to be ran locally as the CI pipeline for Anthropic is not setup. | ||
| private const string SkipReason = "Integrations tests for local execution only"; |
There was a problem hiding this comment.
Typo in the skip reason string: "Integrations tests" should be "Integration tests".
Suggested change
| private const string SkipReason = "Integrations tests for local execution only"; | |
| private const string SkipReason = "Integration tests for local execution only"; |
Comment on lines
16
to
+33
| /// <summary> | ||
| /// Integration tests for Anthropic Skills functionality. | ||
| /// These tests are designed to be run locally with a valid Anthropic API key. | ||
| /// </summary> | ||
| public sealed class AnthropicSkillsIntegrationTests | ||
| { | ||
| // All tests for Anthropic are intended to be ran locally as the CI pipeline for Anthropic is not setup. | ||
| private const string SkipReason = "Integrations tests for local execution only"; | ||
|
|
||
| private static readonly AnthropicConfiguration s_config = TestConfiguration.LoadSection<AnthropicConfiguration>(); | ||
|
|
||
| [Fact(Skip = SkipReason)] | ||
| [Fact] | ||
| public async Task CreateAgentWithPptxSkillAsync() | ||
| { | ||
| Assert.SkipWhen(SkipReason is not null, SkipReason ?? string.Empty); | ||
|
|
||
| // Arrange | ||
| AnthropicClient anthropicClient = new() { ApiKey = s_config.ApiKey }; | ||
| string model = s_config.ChatModelId; | ||
| AnthropicConfiguration config = TestConfiguration.LoadSection<AnthropicConfiguration>(); | ||
| AnthropicClient anthropicClient = new() { ApiKey = config.ApiKey }; | ||
| string model = config.ChatModelId; |
There was a problem hiding this comment.
The file summary says these tests are designed to be run locally with a valid API key, but they currently unconditionally skip because SkipReason is a non-null const. If the goal is “skip in CI but runnable locally without code edits”, consider making the skip conditional (e.g., based on an env var/CI detection or missing API key) and updating the wording accordingly.
| @@ -20,7 +20,6 @@ public class AnthropicChatCompletionFixture : IChatClientAgentFixture | |||
| // All tests for Anthropic are intended to be ran locally as the CI pipeline for Anthropic is not setup. | |||
| internal const string SkipReason = "Integrations tests for local execution only"; | |||
There was a problem hiding this comment.
Typo in the skip reason string: "Integrations tests" should be "Integration tests".
Suggested change
| internal const string SkipReason = "Integrations tests for local execution only"; | |
| internal const string SkipReason = "Integration tests for local execution only"; |
| public async ValueTask InitializeAsync() => | ||
| public async ValueTask InitializeAsync() | ||
| { | ||
| Assert.SkipWhen(SkipReason is not null, SkipReason ?? string.Empty); |
There was a problem hiding this comment.
SkipReason is a non-null const, so the fixture initialization will always skip and the SkipWhen(... is not null, ...) check is redundant. Consider using Assert.Skip(SkipReason) to make the unconditional skip explicit, or make SkipReason/the condition dependent on CI or configuration availability if the fixture should be runnable locally without code changes.
Suggested change
| Assert.SkipWhen(SkipReason is not null, SkipReason ?? string.Empty); | |
| Assert.Skip(SkipReason); |
Comment on lines
+21
to
22
| Assert.SkipWhen(ManualVerification is not null, ManualVerification ?? string.Empty); | ||
| return base.RunWithChatMessageReturnsExpectedResultAsync(); |
There was a problem hiding this comment.
ManualVerification is a constant non-null string, so Assert.SkipWhen(ManualVerification is not null, ...) will always skip these tests and the ?? string.Empty fallback is never used. Consider simplifying to Assert.Skip(ManualVerification) (or make ManualVerification nullable/non-const if you want to toggle skipping without the redundant condition).
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 5, 2026
* .NET: Upgrade to XUnit 3 and Microsoft Testing Platform (#4176) * Fix copilot studio integration tests failure (#4209) * Fix anthropic integration tests and skip reason (#4211) * Remove accidental add of code coverage for integration tests (#4219) * Add solution filtered parallel test run (#4226) * Fix build paths (#4228) * Fix coverage settings path and trait filter (#4229) * Add project name filter to solution (#4231) * Increase Integration Test Parallelism (#4241) * Increase integration tests threads to 4x (#4242) * Separate build and test into parallel jobs (#4243) * Filter src by framework for tests build (#4244) * Separate build and test into parallel jobs * Filter source projects by framework for tests build * Pre-build samples via tests to avoid timeouts (#4245) * Separate build from run for console sample validation (#4251) * Address PR comments (#4255) * Merge and move scripts (#4308) * .NET: Add Microsoft Fabric sample #3674 (#4230) Co-authored-by: Chris <66376200+crickman@users.noreply.github.com> * Python: Phase 2: Embedding clients for Ollama, Bedrock, and Azure AI Inference (#4207) * Phase 2: Embedding clients for Ollama, Bedrock, and Azure AI Inference Add embedding client implementations to existing provider packages: - OllamaEmbeddingClient: Text embeddings via Ollama's embed API - BedrockEmbeddingClient: Text embeddings via Amazon Titan on Bedrock - AzureAIInferenceEmbeddingClient: Text and image embeddings via Azure AI Inference, supporting Content | str input with separate model IDs for text (AZURE_AI_INFERENCE_EMBEDDING_MODEL_ID) and image (AZURE_AI_INFERENCE_IMAGE_EMBEDDING_MODEL_ID) endpoints Additional changes: - Rename EmbeddingCoT -> EmbeddingT, EmbeddingOptionsCoT -> EmbeddingOptionsT - Add otel_provider_name passthrough to all embedding clients - Register integration pytest marker in all packages - Add lazy-loading namespace exports for Ollama and Bedrock embeddings - Add image embedding sample using Cohere-embed-v3-english - Add azure-ai-inference dependency to azure-ai package Part of #1188 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix mypy duplicate name and ruff lint issues - Rename second 'vector' variable to 'img_vector' in image embedding loop - Combine nested with statements in tests - Remove unused result assignments in tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * updates from feedback * Fix CI failures in embedding usage handling - Fix Azure AI embedding mypy issues by normalizing vectors to list[float], safely accumulating optional usage token fields, and filtering None entries before constructing GeneratedEmbeddings - Avoid Bandit false positive by initializing usage details as an empty dict - Update OpenAI embedding tests to assert canonical usage keys (input_token_count/total_token_count) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * [Purview] Mark responses as responses and fix epoch bug for python long overflow (#4225) * .NET: Support InvokeMcpTool for declarative workflows (#4204) * Initial implementation of InvokeMcpTool in declarative workflow * Cleaned up sample implementation * Updated sample comments. * Added missing executor routing attribute * Fix PR comments. * Updated based on PR comments. * Updated based on PR comments. * Removed unnecessary using statement. * Update Python package versions to rc2 (#4258) - Bump core and azure-ai to 1.0.0rc2 - Bump preview packages to 1.0.0b260225 - Update dependencies to >=1.0.0rc2 - Add CHANGELOG entries for changes since rc1 - Update uv.lock Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * .NET: Fixing issue where OpenTelemetry span is never exported in .NET in-process workflow execution (#4196) * 1. Add reproduction test for issue #4155: workflow.run Activity never stopped in streaming OffThread path The WorkflowRunActivity_IsStopped_Streaming_OffThread test demonstrates that the workflow.run OpenTelemetry Activity created in StreamingRunEventStream.RunLoopAsync is started but never stopped when using the OffThread/Default streaming execution. The background run loop keeps running after event consumption completes, so the using Activity? declaration never disposes until explicit StopAsync() is called. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> 2. Fix workflow.run Activity never stopped in streaming OffThread execution (#4155) The workflow.run OpenTelemetry Activity in StreamingRunEventStream.RunLoopAsync was scoped to the method lifetime via 'using'. Since the run loop only exits on cancellation, the Activity was never stopped/exported until explicit disposal. Fix: Remove 'using' and explicitly dispose the Activity when the workflow reaches Idle status (all supersteps complete). A safety-net disposal in the finally block handles cancellation and error paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add root-level workflow.session activity spanning run loop lifetime\n\nImplements two-level telemetry hierarchy per PR feedback from lokitoth:\n- workflow.session: spans the entire run loop / stream lifetime\n- workflow_invoke: per input-to-halt cycle, nested within the session\n\nThis ensures the session activity stays open across multiple turns,\nwhile individual run activities are created and disposed per cycle.\n\nAlso fixes linkedSource CancellationTokenSource disposal leak in\nStreamingRunEventStream (added using declaration)." * Address Copilot review: fix Activity/CTS disposal, rename activity, add error tag\n\n1. LockstepRunEventStream: Remove 'using' from Activity in async iterator\n and manually dispose in finally block (fixes #4155 pattern). Also dispose\n linkedSource CTS in finally to prevent leak.\n2. Tags.cs: Add ErrorMessage (\"error.message\") tag for runtime errors,\n distinct from BuildErrorMessage (\"build.error.message\").\n3. ActivityNames: Rename WorkflowRun from \"workflow_invoke\" to \"workflow.run\"\n for cross-language consistency.\n4. WorkflowTelemetryContext: Fix XML doc to say \"outer/parent span\" instead\n of \"root-level span\".\n5. ObservabilityTests: Assert WorkflowSession absence when DisableWorkflowRun\n is true.\n6. WorkflowRunActivityStopTests: Fix streaming test race by disposing\n StreamingRun before asserting activities are stopped.\n7. StreamingRunEventStream/LockstepRunEventStream: Use Tags.ErrorMessage\n instead of Tags.BuildErrorMessage for runtime error events." * Review fixes: revert workflow_invoke rename, use 'using' for linkedSource, move SessionStarted earlier\n\n- Revert ActivityNames.WorkflowRun back to \"workflow_invoke\" (OTEL semantic convention contract)\n- Use 'using' declaration for linkedSource CTS in LockstepRunEventStream (no timing sensitivity)\n- Move SessionStarted event before WaitForInputAsync in StreamingRunEventStream to match Lockstep behavior" * Improve naming and comments in WorkflowRunActivityStopTests" * Prevent session Activity.Current leak in lockstep mode, add nesting test Save and restore Activity.Current in LockstepRunEventStream.Start() so the session activity doesn't leak into caller code via AsyncLocal. Re-establish Activity.Current = sessionActivity before creating the run activity in TakeEventStreamAsync to preserve parent-child nesting. Add test verifying app activities after RunAsync are not parented under the session, and that the workflow_invoke activity nests under the session." * Fix stale XML doc: WorkflowRun -> WorkflowInvoke in ObservabilityTests --------- Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Python / .NET Samples - Restructure and Improve Samples (Feature Branc… (#4092) * Python: .NET Samples - Restructure and Improve Samples (Feature Branch) (#4091) * Moved by agent (#4094) * Fix readme links * .NET Samples - Create `04-hosting` learning path step (#4098) * Agent move * Agent reorderd * Remove A2A section from README Removed A2A section from the Getting Started README. * Agent fixed links * Fix broken sample links in durable-agents README (#4101) * Initial plan * Fix broken internal links in documentation Co-authored-by: crickman <66376200+crickman@users.noreply.github.com> * Revert template link changes; keep only durable-agents README fix Co-authored-by: crickman <66376200+crickman@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: crickman <66376200+crickman@users.noreply.github.com> * .NET Samples - Create `03-workflows` learning path step (#4102) * Fix solution project path * Python: Fix broken markdown links to repo resources (outside /docs) (#4105) * Initial plan * Fix broken markdown links to repo resources Co-authored-by: crickman <66376200+crickman@users.noreply.github.com> * Update README to rename .NET Workflows Samples section --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: crickman <66376200+crickman@users.noreply.github.com> * .NET Samples - Create `02-agents` learning path step (#4107) * .NET: Fix broken relative link in GroupChatToolApproval README (#4108) * Initial plan * Fix broken link in GroupChatToolApproval README Co-authored-by: crickman <66376200+crickman@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: crickman <66376200+crickman@users.noreply.github.com> * Update labeler configuration for workflow samples * .NET - Reorder Agents samples to start from Step01 instead of Step04 (#4110) * Fix solution * Resolve new sample paths * Move new AgentSkills and AgentWithMemory_Step04 samples * Fix link * Fix readme path * fix: update stale dotnet/samples/Durable path reference in AGENTS.md Co-authored-by: crickman <66376200+crickman@users.noreply.github.com> * Moved new sample * Update solution * Resolve merge (new sample) * Sync to new sample - FoundryAgents_Step21_BingCustomSearch * Updated README * .NET Samples - Configuration Naming Update (#4149) * .NET: Restore AzureFunctions index parity with ConsoleApps under DurableAgents samples (#4221) * Clean-up `05_host_your_agent` * Config setting consistency * Refine samples * AGENTS.md * Move new samples * Re-order samples * Move new project and fixup solution * Fixup model config * Fix up new UT project --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> * Python: Fix Bedrock embedding test stub missing meta attribute (#4287) * Fix Bedrock embedding test stub missing meta attribute * Increase test coverage so gate passes * Python: (ag-ui): fix approval payloads being re-processed on subsequent conversation turns (#4232) * Fix ag-ui tool call issue * Safe json fix * Python: Update workflow orchestration samples to use AzureOpenAIResponsesClient (#4285) * Update workflow orchestration samples to use AzureOpenAIResponsesClient * Fix broken link * Move scripts to scripts folder --------- Co-authored-by: Roger Barreto <19890735+rogerbarreto@users.noreply.github.com> Co-authored-by: Chris <66376200+crickman@users.noreply.github.com> Co-authored-by: Eduard van Valkenburg <eavanvalkenburg@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Rishabh Chawla <rishabhchawla1995@gmail.com> Co-authored-by: Peter Ibekwe <109177538+peibekwe@users.noreply.github.com> Co-authored-by: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com> Co-authored-by: Ben Thomas <ben.thomas@microsoft.com> Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Evan Mattson <35585003+moonbox3@users.noreply.github.com> * Fix encoding (#4309) * Disable Parallelization for WorkflowRunActivityStopTests (#4313) * Revert parallel disable (#4324) * .NET: Disable flakey Workflow Observability tests (#4416) * Disable flakey OffThread test * Disable additional OffThread test * Disable a further test * Disable all observability tests --------- Co-authored-by: Roger Barreto <19890735+rogerbarreto@users.noreply.github.com> Co-authored-by: Chris <66376200+crickman@users.noreply.github.com> Co-authored-by: Eduard van Valkenburg <eavanvalkenburg@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Rishabh Chawla <rishabhchawla1995@gmail.com> Co-authored-by: Peter Ibekwe <109177538+peibekwe@users.noreply.github.com> Co-authored-by: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com> Co-authored-by: Ben Thomas <ben.thomas@microsoft.com> Co-authored-by: alliscode <bentho@microsoft.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Evan Mattson <35585003+moonbox3@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.
Motivation and Context
Description
Contribution Checklist