From d12579ec6690c297b47fb2778845130433aa189b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 19:06:14 +0000 Subject: [PATCH] Strengthen workflow edge helper tests Agent-Logs-Url: https://github.com/microsoft/agent-framework/sessions/af831ee2-0a99-4427-9ffd-a3b5022c1b3b Co-authored-by: lokitoth <6936551+lokitoth@users.noreply.github.com> --- .../SwitchBuilder.cs | 4 + .../WorkflowBuilderExtensions.cs | 5 + .../WorkflowBuilderSmokeTests.cs | 121 +++++++++++++++++- 3 files changed, 126 insertions(+), 4 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/SwitchBuilder.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/SwitchBuilder.cs index 14e6ed4f7c..286329b6ec 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/SwitchBuilder.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/SwitchBuilder.cs @@ -39,6 +39,8 @@ public sealed class SwitchBuilder foreach (ExecutorBinding executor in executors) { + Throw.IfNull(executor, nameof(executors)); + if (!this._executorIndicies.TryGetValue(executor.Id, out int index)) { index = this._executors.Count; @@ -66,6 +68,8 @@ public sealed class SwitchBuilder foreach (ExecutorBinding executor in executors) { + Throw.IfNull(executor, nameof(executors)); + if (!this._executorIndicies.TryGetValue(executor.Id, out int index)) { index = this._executors.Count; diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowBuilderExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowBuilderExtensions.cs index c702cf9ece..f9c3e2056a 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowBuilderExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowBuilderExtensions.cs @@ -52,6 +52,8 @@ public static class WorkflowBuilderExtensions /// The updated instance. public static WorkflowBuilder ForwardMessage(this WorkflowBuilder builder, ExecutorBinding source, IEnumerable targets, Func? condition = null) { + Throw.IfNull(builder); + Throw.IfNull(source); Throw.IfNull(targets); Func predicate = WorkflowBuilder.CreateConditionFunc(IsAllowedTypeAndMatchingCondition)!; @@ -93,6 +95,8 @@ public static class WorkflowBuilderExtensions /// The updated instance with the added edges. public static WorkflowBuilder ForwardExcept(this WorkflowBuilder builder, ExecutorBinding source, IEnumerable targets) { + Throw.IfNull(builder); + Throw.IfNull(source); Throw.IfNull(targets); Func predicate = WorkflowBuilder.CreateConditionFunc((Func)IsAllowedType)!; @@ -129,6 +133,7 @@ public static class WorkflowBuilderExtensions { Throw.IfNull(builder); Throw.IfNull(source); + Throw.IfNull(executors); HashSet seenExecutors = [source.Id]; diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowBuilderSmokeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowBuilderSmokeTests.cs index feb59b35dd..1ca2027446 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowBuilderSmokeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowBuilderSmokeTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. using System; +using System.Collections.Generic; using FluentAssertions; namespace Microsoft.Agents.AI.Workflows.UnitTests; @@ -174,6 +175,7 @@ public partial class WorkflowBuilderSmokeTests Edge edge = GetSingleEdge(workflow, source.Id); edge.Kind.Should().Be(EdgeKind.Direct); edge.DirectEdgeData.Should().NotBeNull(); + edge.DirectEdgeData!.SourceId.Should().Be(source.Id); edge.DirectEdgeData!.SinkId.Should().Be(target.Id); edge.DirectEdgeData.Condition.Should().NotBeNull(); edge.DirectEdgeData.Condition!("message").Should().BeTrue(); @@ -198,6 +200,7 @@ public partial class WorkflowBuilderSmokeTests Edge edge = GetSingleEdge(workflow, source.Id); edge.Kind.Should().Be(EdgeKind.FanOut); edge.FanOutEdgeData.Should().NotBeNull(); + edge.FanOutEdgeData!.SourceId.Should().Be(source.Id); edge.FanOutEdgeData!.SinkIds.Should().Equal([target1.Id, target2.Id]); edge.FanOutEdgeData.EdgeAssigner.Should().NotBeNull(); edge.FanOutEdgeData.EdgeAssigner!("match", 2).Should().Equal([0, 1]); @@ -221,6 +224,7 @@ public partial class WorkflowBuilderSmokeTests Edge edge = GetSingleEdge(workflow, source.Id); edge.Kind.Should().Be(EdgeKind.Direct); edge.DirectEdgeData.Should().NotBeNull(); + edge.DirectEdgeData!.SourceId.Should().Be(source.Id); edge.DirectEdgeData!.SinkId.Should().Be(target.Id); edge.DirectEdgeData.Condition.Should().NotBeNull(); edge.DirectEdgeData.Condition!("message").Should().BeFalse(); @@ -245,6 +249,7 @@ public partial class WorkflowBuilderSmokeTests Edge edge = GetSingleEdge(workflow, source.Id); edge.Kind.Should().Be(EdgeKind.FanOut); edge.FanOutEdgeData.Should().NotBeNull(); + edge.FanOutEdgeData!.SourceId.Should().Be(source.Id); edge.FanOutEdgeData!.SinkIds.Should().Equal([target1.Id, target2.Id]); edge.FanOutEdgeData.EdgeAssigner.Should().NotBeNull(); edge.FanOutEdgeData.EdgeAssigner!(42, 2).Should().Equal([0, 1]); @@ -265,8 +270,15 @@ public partial class WorkflowBuilderSmokeTests .Build(); // Assert - GetSingleEdge(workflow, source.Id).DirectEdgeData!.SinkId.Should().Be(middle.Id); - GetSingleEdge(workflow, middle.Id).DirectEdgeData!.SinkId.Should().Be(end.Id); + Edge firstEdge = GetSingleEdge(workflow, source.Id); + firstEdge.Kind.Should().Be(EdgeKind.Direct); + firstEdge.DirectEdgeData!.SourceId.Should().Be(source.Id); + firstEdge.DirectEdgeData.SinkId.Should().Be(middle.Id); + + Edge secondEdge = GetSingleEdge(workflow, middle.Id); + secondEdge.Kind.Should().Be(EdgeKind.Direct); + secondEdge.DirectEdgeData!.SourceId.Should().Be(middle.Id); + secondEdge.DirectEdgeData.SinkId.Should().Be(end.Id); } [Fact] @@ -302,8 +314,16 @@ public partial class WorkflowBuilderSmokeTests workflow.Ports[PortId].Request.Should().Be(typeof(string)); workflow.Ports[PortId].Response.Should().Be(typeof(int)); workflow.ExecutorBindings.Should().ContainKey(PortId); - GetSingleEdge(workflow, source.Id).DirectEdgeData!.SinkId.Should().Be(PortId); - GetSingleEdge(workflow, PortId).DirectEdgeData!.SinkId.Should().Be(source.Id); + + Edge requestEdge = GetSingleEdge(workflow, source.Id); + requestEdge.Kind.Should().Be(EdgeKind.Direct); + requestEdge.DirectEdgeData!.SourceId.Should().Be(source.Id); + requestEdge.DirectEdgeData.SinkId.Should().Be(PortId); + + Edge responseEdge = GetSingleEdge(workflow, PortId); + responseEdge.Kind.Should().Be(EdgeKind.Direct); + responseEdge.DirectEdgeData!.SourceId.Should().Be(PortId); + responseEdge.DirectEdgeData.SinkId.Should().Be(source.Id); } [Fact] @@ -327,6 +347,7 @@ public partial class WorkflowBuilderSmokeTests Edge edge = GetSingleEdge(workflow, source.Id); edge.Kind.Should().Be(EdgeKind.FanOut); edge.FanOutEdgeData.Should().NotBeNull(); + edge.FanOutEdgeData!.SourceId.Should().Be(source.Id); edge.FanOutEdgeData!.SinkIds.Should().Equal([stringTarget.Id, intTarget.Id, defaultTarget.Id]); edge.FanOutEdgeData.EdgeAssigner.Should().NotBeNull(); edge.FanOutEdgeData.EdgeAssigner!("match", 3).Should().Equal([0]); @@ -334,6 +355,98 @@ public partial class WorkflowBuilderSmokeTests edge.FanOutEdgeData.EdgeAssigner!("other", 3).Should().Equal([2]); } + [Fact] + public void ForwardMessage_InvalidArguments_Throw() + { + // Arrange + WorkflowBuilder builder = new("start"); + NoOpExecutor source = new("start"); + NoOpExecutor target = new("target"); + + // Act/Assert + Assert.Throws("builder", () => ((WorkflowBuilder)null!).ForwardMessage(source, target)); + Assert.Throws("source", () => builder.ForwardMessage(null!, target)); + Assert.Throws("target", () => builder.ForwardMessage(source, (ExecutorBinding)null!)); + Assert.Throws("targets", () => builder.ForwardMessage(source, (IEnumerable)null!)); + Assert.Throws("executors", () => builder.ForwardMessage(source, [target, null!])); + Assert.Throws("targets", () => builder.ForwardMessage(source, [])); + } + + [Fact] + public void ForwardExcept_InvalidArguments_Throw() + { + // Arrange + WorkflowBuilder builder = new("start"); + NoOpExecutor source = new("start"); + NoOpExecutor target = new("target"); + + // Act/Assert + Assert.Throws("builder", () => ((WorkflowBuilder)null!).ForwardExcept(source, target)); + Assert.Throws("source", () => builder.ForwardExcept(null!, target)); + Assert.Throws("target", () => builder.ForwardExcept(source, (ExecutorBinding)null!)); + Assert.Throws("targets", () => builder.ForwardExcept(source, (IEnumerable)null!)); + Assert.Throws("executors", () => builder.ForwardExcept(source, [target, null!])); + Assert.Throws("targets", () => builder.ForwardExcept(source, [])); + } + + [Fact] + public void AddChain_InvalidArguments_Throw() + { + // Arrange + WorkflowBuilder builder = new("start"); + NoOpExecutor source = new("start"); + NoOpExecutor target = new("target"); + + // Act/Assert + Assert.Throws("builder", () => ((WorkflowBuilder)null!).AddChain(source, [target])); + Assert.Throws("source", () => builder.AddChain(null!, [target])); + Assert.Throws("executors", () => builder.AddChain(source, null!)); + Assert.Throws("executors", () => builder.AddChain(source, [target, null!])); + Assert.Throws("executors", () => builder.AddChain(source, [target, source])); + } + + [Fact] + public void AddExternalCall_InvalidArguments_Throw() + { + // Arrange + WorkflowBuilder builder = new("start"); + NoOpExecutor source = new("start"); + + // Act/Assert + Assert.Throws("builder", () => ((WorkflowBuilder)null!).AddExternalCall(source, "port")); + Assert.Throws("source", () => builder.AddExternalCall(null!, "port")); + Assert.Throws("portId", () => builder.AddExternalCall(source, null!)); + } + + [Fact] + public void AddSwitch_InvalidArguments_Throw() + { + // Arrange + WorkflowBuilder builder = new("start"); + NoOpExecutor source = new("start"); + + // Act/Assert + Assert.Throws("builder", () => ((WorkflowBuilder)null!).AddSwitch(source, _ => { })); + Assert.Throws("source", () => builder.AddSwitch(null!, _ => { })); + Assert.Throws("configureSwitch", () => builder.AddSwitch(source, null!)); + Assert.Throws("targets", () => builder.AddSwitch(source, _ => { })); + } + + [Fact] + public void SwitchBuilder_InvalidArguments_Throw() + { + // Arrange + SwitchBuilder switchBuilder = new(); + NoOpExecutor target = new("target"); + + // Act/Assert + Assert.Throws("predicate", () => switchBuilder.AddCase(null!, [target])); + Assert.Throws("executors", () => switchBuilder.AddCase(_ => true, null!)); + Assert.Throws("executors", () => switchBuilder.AddCase(_ => true, [target, null!])); + Assert.Throws("executors", () => switchBuilder.WithDefault(null!)); + Assert.Throws("executors", () => switchBuilder.WithDefault([target, null!])); + } + private static Edge GetSingleEdge(Workflow workflow, string sourceId) => workflow.Edges[sourceId].Should().ContainSingle().Subject; }