#66 : Unit test for HasAppPort, other cleanup

man-group · Nov 3, 2024 · d5fd26d · d5fd26d
1 parent e4f7886
commit d5fd26d
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 5 deletions.
diff --git a/samples/Workflow/ConsoleSample/WorkflowConsoleApp/Program.cs b/samples/Workflow/ConsoleSample/WorkflowConsoleApp/Program.cs
@@ -39,6 +39,8 @@
 
 // Dapr uses a random port for gRPC by default. If we don't know what that port
 // is (because this app was started separate from dapr), then assume 4001.
+// For Workflows we need to use the Environment Variable rather than a Sidekick appsettings.json entry
+// because the Workflow client can be initialized before Sidekick has started.
 if (string.IsNullOrEmpty(Environment.GetEnvironmentVariable("DAPR_GRPC_PORT")))
 {
     Environment.SetEnvironmentVariable("DAPR_GRPC_PORT", "4001");

diff --git a/src/Man.Dapr.Sidekick/DaprConstants.cs b/src/Man.Dapr.Sidekick/DaprConstants.cs
@@ -28,7 +28,6 @@ public class DaprConstants
         public const string DaprCertKeyEnvironmentVariable = "DAPR_CERT_KEY";
         public const string DaprGrpcPortEnvironmentVariable = "DAPR_GRPC_PORT";
         public const string DaprHttpPortEnvironmentVariable = "DAPR_HTTP_PORT";
-        public const string DaprMetricsPortEnvironmentVariable = "DAPR_METRICS_PORT";
         public const string DaprPlacementHealthPortEnvironmentVariable = "DAPR_PLACEMENT_HEALTH_PORT";
         public const string DaprPlacementMetricsPortEnvironmentVariable = "DAPR_PLACEMENT_METRICS_PORT";
         public const string DaprPlacementPortEnvironmentVariable = "DAPR_PLACEMENT_PORT";

diff --git a/src/Man.Dapr.Sidekick/Process/DaprSidecarProcess.cs b/src/Man.Dapr.Sidekick/Process/DaprSidecarProcess.cs
@@ -87,8 +87,8 @@ protected override void AssignPorts(PortAssignmentBuilder<DaprSidecarOptions> bu
                 .Add(x => x.AppPort, 8500, DaprConstants.DaprAppPortEnvironmentVariable)
                 .Add(x => x.DaprGrpcPort, 50001, DaprConstants.DaprGrpcPortEnvironmentVariable)
                 .Add(x => x.DaprHttpPort, 3500, DaprConstants.DaprHttpPortEnvironmentVariable)
-                .Add(x => x.MetricsPort, 9090, DaprConstants.DaprMetricsPortEnvironmentVariable)
-                .Add(x => x.ProfilePort, 7777, DaprConstants.DaprProfilePortEnvironmentVariable);
+                .Add(x => x.MetricsPort, 9090)
+                .Add(x => x.ProfilePort, 7777);
 
         protected override void AssignLocations(DaprSidecarOptions options, string daprFolder)
         {
@@ -145,12 +145,11 @@ protected override void AddCommandLineArguments(DaprSidecarOptions source, Comma
         protected override void AddEnvironmentVariables(DaprSidecarOptions source, EnvironmentVariableBuilder builder) => builder
             .Add(DaprConstants.AppApiTokenEnvironmentVariable, source.AppApiToken)
             .Add(DaprConstants.DaprApiTokenEnvironmentVariable, source.DaprApiToken)
-            .Add(DaprConstants.DaprAppPortEnvironmentVariable, source.AppPort)
+            .Add(DaprConstants.DaprAppPortEnvironmentVariable, source.AppPort, () => source.HasAppPort != false)
             .Add(DaprConstants.DaprCertChainEnvironmentVariable, source.IssuerCertificate)
             .Add(DaprConstants.DaprCertKeyEnvironmentVariable, source.IssuerKey)
             .Add(DaprConstants.DaprGrpcPortEnvironmentVariable, source.DaprGrpcPort)
             .Add(DaprConstants.DaprHttpPortEnvironmentVariable, source.DaprHttpPort)
-            .Add(DaprConstants.DaprMetricsPortEnvironmentVariable, source.MetricsPort)
             .Add(DaprConstants.DaprProfilePortEnvironmentVariable, source.ProfilePort, () => source.Profiling == true)
             .Add(DaprConstants.DaprTrustAnchorsEnvironmentVariable, source.TrustAnchorsCertificate)
             .Add(DaprConstants.NamespaceEnvironmentVariable, source.Namespace);

diff --git a/tests/Man.Dapr.Sidekick.Tests/Options/DaprSidecarOptionsTests.cs b/tests/Man.Dapr.Sidekick.Tests/Options/DaprSidecarOptionsTests.cs
@@ -139,6 +139,7 @@ private static void Compare(DaprSidecarOptions source, DaprSidecarOptions target
             Assert.That(target.DaprHttpMaxRequestSize, Is.EqualTo(source.DaprHttpMaxRequestSize));
             Assert.That(target.DaprHttpPort, Is.EqualTo(source.DaprHttpPort));
             Assert.That(target.DaprInternalGrpcPort, Is.EqualTo(source.DaprInternalGrpcPort));
+            Assert.That(target.HasAppPort, Is.EqualTo(source.HasAppPort));
             Assert.That(target.KubeConfig, Is.EqualTo(source.KubeConfig));
             Assert.That(target.MetricsPort, Is.EqualTo(source.MetricsPort));
             Assert.That(target.Mode, Is.EqualTo(source.Mode));
@@ -172,6 +173,7 @@ private static void Compare(DaprSidecarOptions source, DaprSidecarOptions target
             DaprHttpMaxRequestSize = 400,
             DaprHttpPort = 500,
             DaprInternalGrpcPort = 600,
+            HasAppPort = true,
             KubeConfig = "KubeConfig",
             MetricsPort = 700,
             Mode = "Mode",

diff --git a/tests/Man.Dapr.Sidekick.Tests/Process/DaprSidecarProcessTests.cs b/tests/Man.Dapr.Sidekick.Tests/Process/DaprSidecarProcessTests.cs
@@ -196,6 +196,31 @@ public void Should_not_add_empty_sentry_address()
                 Assert.That(builder.ToString(), Does.Not.Contain("sentry-address"));
             }
 
+            [TestCase(null)]
+            [TestCase(false)]
+            [TestCase(true)]
+            public void Should_suppress_appport(bool? hasAppPort)
+            {
+                var p = new MockDaprSidecarProcess();
+                var builder = new CommandLineArgumentBuilder();
+                var options = new DaprSidecarOptions
+                {
+                    HasAppPort = hasAppPort,
+                    AppPort = 1234
+                };
+
+                p.AddCommandLineArguments(options, builder);
+
+                if (hasAppPort == false)
+                {
+                    Assert.That(builder.ToString(), Does.Not.Contain("app-port"));
+                }
+                else
+                {
+                    Assert.That(builder.ToString(), Does.Contain("app-port"));
+                }
+            }
+
             [Test]
             public void Should_add_all_arguments()
             {