From cb2f418f44276e50c03ed9a2cbdec8516ce3b3fb Mon Sep 17 00:00:00 2001 From: glopesdev Date: Mon, 15 Jul 2024 13:06:26 +0100 Subject: [PATCH 1/5] Refactor subject tests to use new test helpers --- Bonsai.Core.Tests/SubjectTests.cs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/Bonsai.Core.Tests/SubjectTests.cs b/Bonsai.Core.Tests/SubjectTests.cs index f590dc6c..c231facf 100644 --- a/Bonsai.Core.Tests/SubjectTests.cs +++ b/Bonsai.Core.Tests/SubjectTests.cs @@ -22,13 +22,12 @@ public override IObservable Process(IObservable source) [ExpectedException(typeof(WorkflowBuildException))] public void Build_MulticastInterfaceToSubjectOfDifferentInterface_ThrowsBuildException() { - var builder = new WorkflowBuilder(); - builder.Workflow.Add(new BehaviorSubject { Name = nameof(BehaviorSubject) }); - var source = builder.Workflow.Add(new CombinatorBuilder { Combinator = new DoubleProperty { Value = 5.5 } }); - var convert1 = builder.Workflow.Add(new CombinatorBuilder { Combinator = new TypeCombinatorMock() }); - var convert2 = builder.Workflow.Add(new MulticastSubject { Name = nameof(BehaviorSubject) }); - builder.Workflow.AddEdge(source, convert1, new ExpressionBuilderArgument()); - builder.Workflow.AddEdge(convert1, convert2, new ExpressionBuilderArgument()); + var builder = new TestWorkflow() + .Append(new BehaviorSubject { Name = nameof(BehaviorSubject) }) + .ResetCursor() + .AppendCombinator(new DoubleProperty { Value = 5.5 }) + .AppendCombinator(new TypeCombinatorMock()) + .Append(new MulticastSubject { Name = nameof(BehaviorSubject) }); var expression = builder.Workflow.Build(); Assert.IsNotNull(expression); } @@ -36,12 +35,10 @@ public void Build_MulticastInterfaceToSubjectOfDifferentInterface_ThrowsBuildExc [TestMethod] public void ResourceSubject_SourceTerminatesExceptionally_ShouldNotTryToDispose() { - var workflowBuilder = new WorkflowBuilder(); - var source = workflowBuilder.Workflow.Add(new CombinatorBuilder { Combinator = new ThrowSource() }); - var subject = workflowBuilder.Workflow.Add(new ResourceSubject { Name = nameof(ResourceSubject) }); - var sink = workflowBuilder.Workflow.Add(new CombinatorBuilder { Combinator = new CatchSink() }); - workflowBuilder.Workflow.AddEdge(source, subject, new ExpressionBuilderArgument()); - workflowBuilder.Workflow.AddEdge(subject, sink, new ExpressionBuilderArgument()); + var workflowBuilder = new TestWorkflow() + .AppendCombinator(new ThrowSource()) + .Append(new ResourceSubject { Name = nameof(ResourceSubject) }) + .AppendCombinator(new CatchSink()); var observable = workflowBuilder.Workflow.BuildObservable(); observable.FirstOrDefaultAsync().Wait(); } @@ -58,7 +55,7 @@ class CatchSink : Sink { public override IObservable Process(IObservable source) { - return source.Catch(Observable.Empty()); + return source.Catch(Observable.Empty()); } } From 9fabe1396bf698a02c239d6fa0e6c0177ac51bc1 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Mon, 15 Jul 2024 13:08:31 +0100 Subject: [PATCH 2/5] Add regression testing for multicast subject --- Bonsai.Core.Tests/SubjectTests.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Bonsai.Core.Tests/SubjectTests.cs b/Bonsai.Core.Tests/SubjectTests.cs index c231facf..d958fd69 100644 --- a/Bonsai.Core.Tests/SubjectTests.cs +++ b/Bonsai.Core.Tests/SubjectTests.cs @@ -32,6 +32,21 @@ public void Build_MulticastInterfaceToSubjectOfDifferentInterface_ThrowsBuildExc Assert.IsNotNull(expression); } + [TestMethod] + public void Build_MulticastSourceToObjectSubject_PreservesTypeOfSourceSequence() + { + // related to https://github.com/bonsai-rx/bonsai/issues/1914 + var workflow = new TestWorkflow() + .Append(new BehaviorSubject { Name = nameof(BehaviorSubject) }) + .ResetCursor() + .AppendCombinator(new IntProperty()) + .Append(new MulticastSubject { Name = nameof(BehaviorSubject) }) + .AppendOutput() + .Workflow; + var expression = workflow.Build(); + Assert.AreEqual(typeof(IObservable), expression.Type); + } + [TestMethod] public void ResourceSubject_SourceTerminatesExceptionally_ShouldNotTryToDispose() { From 48a8e99422ac5d2fd7646caf85d992bc0456e00c Mon Sep 17 00:00:00 2001 From: glopesdev Date: Mon, 15 Jul 2024 13:11:58 +0100 Subject: [PATCH 3/5] Ensure multicast subject preserves source type --- Bonsai.Core/Expressions/MulticastSubject.cs | 23 +++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/Bonsai.Core/Expressions/MulticastSubject.cs b/Bonsai.Core/Expressions/MulticastSubject.cs index 44a9fe08..0b984732 100644 --- a/Bonsai.Core/Expressions/MulticastSubject.cs +++ b/Bonsai.Core/Expressions/MulticastSubject.cs @@ -67,8 +67,16 @@ public override Expression Build(IEnumerable arguments) ); } - source = CoerceMethodArgument(typeof(IObservable<>).MakeGenericType(subjectType), source); - observableType = subjectType; + var conversionParameter = Expression.Parameter(observableType); + var conversionBody = Expression.Convert(conversionParameter, subjectType); + var conversion = Expression.Lambda(conversionBody, conversionParameter); + return Expression.Call( + typeof(MulticastSubject), + nameof(Process), + new[] { observableType, subjectType }, + source, + subjectExpression, + conversion); } return Expression.Call(typeof(MulticastSubject), nameof(Process), new[] { observableType }, source, subjectExpression); @@ -79,6 +87,17 @@ static IObservable Process(IObservable source, IObser return source.Do(subject); } + static IObservable Process( + IObservable source, + IObserver subject, + Func conversion) + { + return source.Do( + value => subject.OnNext(conversion(value)), + subject.OnError, + subject.OnCompleted); + } + class MulticastSubjectTypeDescriptionProvider : TypeDescriptionProvider { readonly MulticastSubjectTypeDescriptor typeDescriptor = new MulticastSubjectTypeDescriptor(null); From 49f2eac43ab17bd134323a7c224cddf394bb970e Mon Sep 17 00:00:00 2001 From: glopesdev Date: Mon, 15 Jul 2024 13:48:50 +0100 Subject: [PATCH 4/5] Allow building typed test observable sequence --- Bonsai.Core.Tests/TestWorkflow.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Bonsai.Core.Tests/TestWorkflow.cs b/Bonsai.Core.Tests/TestWorkflow.cs index 27a5dac1..a81ec258 100644 --- a/Bonsai.Core.Tests/TestWorkflow.cs +++ b/Bonsai.Core.Tests/TestWorkflow.cs @@ -1,4 +1,5 @@ using System; +using System.Linq.Expressions; using Bonsai.Dag; using Bonsai.Expressions; @@ -97,5 +98,12 @@ public ExpressionBuilderGraph ToInspectableGraph() { return Workflow.ToInspectableGraph(); } + + public IObservable BuildObservable() + { + var expression = Workflow.Build(); + var observableFactory = Expression.Lambda>>(expression).Compile(); + return observableFactory(); + } } } From 458143d3e3a529d9460169bae3112102c1460e76 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Mon, 15 Jul 2024 14:12:34 +0100 Subject: [PATCH 5/5] Add missing test coverage for multicast subject --- Bonsai.Core.Tests/SubjectTests.cs | 58 ++++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 5 deletions(-) diff --git a/Bonsai.Core.Tests/SubjectTests.cs b/Bonsai.Core.Tests/SubjectTests.cs index d958fd69..3a4d0ae4 100644 --- a/Bonsai.Core.Tests/SubjectTests.cs +++ b/Bonsai.Core.Tests/SubjectTests.cs @@ -1,5 +1,8 @@ using System; +using System.Collections.Generic; +using System.Linq.Expressions; using System.Reactive.Linq; +using System.Threading.Tasks; using Bonsai.Expressions; using Bonsai.Reactive; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -18,6 +21,38 @@ public override IObservable Process(IObservable source) } } + class ConstantExpressionBuilder : ZeroArgumentExpressionBuilder + { + public Expression Expression { get; set; } + + public override Expression Build(IEnumerable arguments) + { + return Expression; + } + } + + [TestMethod] + [ExpectedException(typeof(InvalidOperationException))] + public void Build_MulticastSubjectMissingBuildContext_ThrowsBuildException() + { + var source = new UnitBuilder().Build(); + var builder = new MulticastSubject { Name = nameof(BehaviorSubject) }; + builder.Build(source); + Assert.Fail(); + } + + [TestMethod] + public void Build_MulticastSubjectMissingName_ReturnsSameSequence() + { + var source = Expression.Constant(Observable.Return(0)); + var builder = new TestWorkflow() + .Append(new ConstantExpressionBuilder { Expression = source }) + .Append(new MulticastSubject()) + .AppendOutput(); + var expression = builder.Workflow.Build(); + Assert.AreSame(source, expression); + } + [TestMethod] [ExpectedException(typeof(WorkflowBuildException))] public void Build_MulticastInterfaceToSubjectOfDifferentInterface_ThrowsBuildException() @@ -33,7 +68,21 @@ public void Build_MulticastInterfaceToSubjectOfDifferentInterface_ThrowsBuildExc } [TestMethod] - public void Build_MulticastSourceToObjectSubject_PreservesTypeOfSourceSequence() + public async Task Build_MulticastSourceToSubject_ReturnsSameValue() + { + var value = 32; + var workflow = new TestWorkflow() + .Append(new BehaviorSubject { Name = nameof(BehaviorSubject) }) + .ResetCursor() + .AppendCombinator(new IntProperty { Value = value }) + .Append(new MulticastSubject { Name = nameof(BehaviorSubject) }) + .AppendOutput(); + var observable = workflow.BuildObservable(); + Assert.AreEqual(value, await observable.Take(1)); + } + + [TestMethod] + public async Task Build_MulticastSourceToObjectSubject_PreservesTypeOfSourceSequence() { // related to https://github.com/bonsai-rx/bonsai/issues/1914 var workflow = new TestWorkflow() @@ -41,10 +90,9 @@ public void Build_MulticastSourceToObjectSubject_PreservesTypeOfSourceSequence() .ResetCursor() .AppendCombinator(new IntProperty()) .Append(new MulticastSubject { Name = nameof(BehaviorSubject) }) - .AppendOutput() - .Workflow; - var expression = workflow.Build(); - Assert.AreEqual(typeof(IObservable), expression.Type); + .AppendOutput(); + var observable = workflow.BuildObservable(); + Assert.AreEqual(0, await observable.Take(1)); } [TestMethod]