From 03d19b066011bb1bcdfa3e9d4d07f9afac713cd2 Mon Sep 17 00:00:00 2001 From: Ben Pearce Date: Fri, 22 May 2026 15:43:48 +1000 Subject: [PATCH 1/4] ensure that the environment variables collection is never null --- .../Features/Scripting/ScriptEngine.cs | 11 +++- .../Scripting/ScriptEngineFixture.cs | 60 ++++++++++++++++++- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/source/Calamari.Common/Features/Scripting/ScriptEngine.cs b/source/Calamari.Common/Features/Scripting/ScriptEngine.cs index 3d5232a2a0..8d97f75d75 100644 --- a/source/Calamari.Common/Features/Scripting/ScriptEngine.cs +++ b/source/Calamari.Common/Features/Scripting/ScriptEngine.cs @@ -7,7 +7,6 @@ using Calamari.Common.Features.Scripting.Python; using Calamari.Common.Features.Scripting.WindowsPowerShell; using Calamari.Common.Features.Scripts; -using Calamari.Common.FeatureToggles; using Calamari.Common.Plumbing.Extensions; using Calamari.Common.Plumbing.Logging; using Calamari.Common.Plumbing.Variables; @@ -18,11 +17,15 @@ public interface IScriptEngine { ScriptSyntax[] GetSupportedTypes(); + CommandResult Execute(Script script, + IVariables variables, + ICommandLineRunner commandLineRunner); + CommandResult Execute( Script script, IVariables variables, ICommandLineRunner commandLineRunner, - Dictionary? environmentVars = null); + Dictionary environmentVars); } public class ScriptEngine : IScriptEngine @@ -41,11 +44,13 @@ public ScriptSyntax[] GetSupportedTypes() return ScriptSyntaxHelper.GetPreferenceOrderedScriptSyntaxesForEnvironment(); } + public CommandResult Execute(Script script, IVariables variables, ICommandLineRunner commandLineRunner) => Execute(script, variables, commandLineRunner, []); + public CommandResult Execute( Script script, IVariables variables, ICommandLineRunner commandLineRunner, - Dictionary? environmentVars = null) + Dictionary environmentVars) { var syntax = script.File.ToScriptType(); return BuildWrapperChain(syntax, variables, commandLineRunner) diff --git a/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs b/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs index 9596bd02c9..e170a4426e 100644 --- a/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs +++ b/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs @@ -1,9 +1,10 @@ using System; using System.Collections.Generic; +using Calamari.Common.Features.Processes; using Calamari.Common.Features.Scripting; -using Calamari.Common.Features.Scripting.DotnetScript; using Calamari.Common.Features.Scripts; using Calamari.Common.Plumbing.Logging; +using Calamari.Common.Plumbing.Variables; using Calamari.Testing.Helpers; using FluentAssertions; using NSubstitute; @@ -43,6 +44,63 @@ public void DeterminesCorrectScriptTypePreferencesOrderNonWindows() { DeterminesCorrectScriptTypePreferenceOrder(ScriptPreferencesNonWindows); } + + [Test] + public void ExecuteWithoutEnvironmentVarsPassesEmptyDictionaryToWrappers() + { + Dictionary receivedEnvironmentVars = null; + var capturingWrapper = new CapturingScriptWrapper(envVars => receivedEnvironmentVars = envVars); + + var engine = new ScriptEngine(new List { capturingWrapper }, Substitute.For()); + var variables = new CalamariVariables(); + var runner = Substitute.For(); + + engine.Execute(new Script("test.ps1"), variables, runner); + + receivedEnvironmentVars.Should().NotBeNull(); + receivedEnvironmentVars.Should().BeEmpty(); + } + + [Test] + public void ExecuteWithEnvironmentVarsPassesDictionaryToWrappers() + { + Dictionary receivedEnvironmentVars = null; + var capturingWrapper = new CapturingScriptWrapper(envVars => receivedEnvironmentVars = envVars); + + var engine = new ScriptEngine(new List { capturingWrapper }, Substitute.For()); + var variables = new CalamariVariables(); + var runner = Substitute.For(); + var envVars = new Dictionary { { "AWS_REGION", "us-east-1" } }; + + engine.Execute(new Script("test.ps1"), variables, runner, envVars); + + receivedEnvironmentVars.Should().NotBeNull(); + receivedEnvironmentVars.Should().ContainKey("AWS_REGION"); + } + + /// + /// A wrapper that captures the environmentVars it receives, to verify they are never null. + /// + class CapturingScriptWrapper : IScriptWrapper + { + readonly Action> onExecute; + + public CapturingScriptWrapper(Action> onExecute) + { + this.onExecute = onExecute; + } + + public int Priority => ScriptWrapperPriorities.ToolConfigPriority; + public IScriptWrapper NextWrapper { get; set; } + + public bool IsEnabled(ScriptSyntax syntax) => true; + + public CommandResult ExecuteScript(Script script, ScriptSyntax scriptSyntax, ICommandLineRunner commandLineRunner, Dictionary environmentVars) + { + onExecute(environmentVars); + return new CommandResult("captured", 0); + } + } void DeterminesCorrectScriptTypePreferenceOrder(IEnumerable expected) { From e87494dcda64be4fa19db656ea6e47369c6cf0db Mon Sep 17 00:00:00 2001 From: Ben Pearce Date: Fri, 22 May 2026 16:13:02 +1000 Subject: [PATCH 2/4] fix dependency issue on test class --- .../Scripting/ScriptEngineFixture.cs | 113 +++++++++--------- 1 file changed, 57 insertions(+), 56 deletions(-) diff --git a/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs b/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs index e170a4426e..fd7eedf6d4 100644 --- a/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs +++ b/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs @@ -44,63 +44,64 @@ public void DeterminesCorrectScriptTypePreferencesOrderNonWindows() { DeterminesCorrectScriptTypePreferenceOrder(ScriptPreferencesNonWindows); } - + + [Test] + public void ExecuteWithoutEnvironmentVarsPassesEmptyDictionaryToWrappers() + { + Dictionary receivedEnvironmentVars = null; + var capturingWrapper = new CapturingScriptWrapper() + { + OnExecute = envVars => receivedEnvironmentVars = envVars + }; + + var engine = new ScriptEngine(new List { capturingWrapper }, Substitute.For()); + var variables = new CalamariVariables(); + var runner = Substitute.For(); + + engine.Execute(new Script("test.ps1"), variables, runner); + + receivedEnvironmentVars.Should().NotBeNull(); + receivedEnvironmentVars.Should().BeEmpty(); + } + [Test] - public void ExecuteWithoutEnvironmentVarsPassesEmptyDictionaryToWrappers() - { - Dictionary receivedEnvironmentVars = null; - var capturingWrapper = new CapturingScriptWrapper(envVars => receivedEnvironmentVars = envVars); - - var engine = new ScriptEngine(new List { capturingWrapper }, Substitute.For()); - var variables = new CalamariVariables(); - var runner = Substitute.For(); - - engine.Execute(new Script("test.ps1"), variables, runner); - - receivedEnvironmentVars.Should().NotBeNull(); - receivedEnvironmentVars.Should().BeEmpty(); - } - - [Test] - public void ExecuteWithEnvironmentVarsPassesDictionaryToWrappers() - { - Dictionary receivedEnvironmentVars = null; - var capturingWrapper = new CapturingScriptWrapper(envVars => receivedEnvironmentVars = envVars); - - var engine = new ScriptEngine(new List { capturingWrapper }, Substitute.For()); - var variables = new CalamariVariables(); - var runner = Substitute.For(); - var envVars = new Dictionary { { "AWS_REGION", "us-east-1" } }; - - engine.Execute(new Script("test.ps1"), variables, runner, envVars); - - receivedEnvironmentVars.Should().NotBeNull(); - receivedEnvironmentVars.Should().ContainKey("AWS_REGION"); - } - - /// - /// A wrapper that captures the environmentVars it receives, to verify they are never null. - /// - class CapturingScriptWrapper : IScriptWrapper - { - readonly Action> onExecute; - - public CapturingScriptWrapper(Action> onExecute) - { - this.onExecute = onExecute; - } - - public int Priority => ScriptWrapperPriorities.ToolConfigPriority; - public IScriptWrapper NextWrapper { get; set; } - - public bool IsEnabled(ScriptSyntax syntax) => true; - - public CommandResult ExecuteScript(Script script, ScriptSyntax scriptSyntax, ICommandLineRunner commandLineRunner, Dictionary environmentVars) - { - onExecute(environmentVars); - return new CommandResult("captured", 0); - } - } + public void ExecuteWithEnvironmentVarsPassesDictionaryToWrappers() + { + Dictionary receivedEnvironmentVars = null; + var capturingWrapper = new CapturingScriptWrapper() + { + OnExecute = envVars => receivedEnvironmentVars = envVars + }; + + var engine = new ScriptEngine(new List { capturingWrapper }, Substitute.For()); + var variables = new CalamariVariables(); + var runner = Substitute.For(); + var envVars = new Dictionary { { "AWS_REGION", "us-east-1" } }; + + engine.Execute(new Script("test.ps1"), variables, runner, envVars); + + receivedEnvironmentVars.Should().NotBeNull(); + receivedEnvironmentVars.Should().ContainKey("AWS_REGION"); + } + + /// + /// A wrapper that captures the environmentVars it receives, to verify they are never null. + /// + class CapturingScriptWrapper : IScriptWrapper + { + public Action> OnExecute { get; set; } + + public int Priority => ScriptWrapperPriorities.ToolConfigPriority; + public IScriptWrapper NextWrapper { get; set; } + + public bool IsEnabled(ScriptSyntax syntax) => true; + + public CommandResult ExecuteScript(Script script, ScriptSyntax scriptSyntax, ICommandLineRunner commandLineRunner, Dictionary environmentVars) + { + OnExecute?.Invoke(environmentVars); + return new CommandResult("captured", 0); + } + } void DeterminesCorrectScriptTypePreferenceOrder(IEnumerable expected) { From 7af64c4a40ab073a6cf2fe8e976bb1c7b28271c2 Mon Sep 17 00:00:00 2001 From: Ben Pearce Date: Mon, 25 May 2026 11:13:45 +1000 Subject: [PATCH 3/4] include capturing wrapper in script wrapper chain --- .../Fixtures/Integration/Scripting/ScriptEngineFixture.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs b/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs index fd7eedf6d4..30aa20cbc4 100644 --- a/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs +++ b/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs @@ -99,7 +99,7 @@ class CapturingScriptWrapper : IScriptWrapper public CommandResult ExecuteScript(Script script, ScriptSyntax scriptSyntax, ICommandLineRunner commandLineRunner, Dictionary environmentVars) { OnExecute?.Invoke(environmentVars); - return new CommandResult("captured", 0); + return NextWrapper?.ExecuteScript(script, scriptSyntax, commandLineRunner, environmentVars) ?? new CommandResult("captured", 0); } } From 79c89e27d5fe4478cf6e954d85d04fa38c1d7ed3 Mon Sep 17 00:00:00 2001 From: Ben Pearce Date: Mon, 25 May 2026 12:34:09 +1000 Subject: [PATCH 4/4] test fix --- .../Integration/Scripting/ScriptEngineFixture.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs b/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs index 30aa20cbc4..cbcffa1bca 100644 --- a/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs +++ b/source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs @@ -31,6 +31,13 @@ public class ScriptEngineFixture ScriptSyntax.Bash }; + ICommandLineRunner runner; + public ScriptEngineFixture() + { + runner = Substitute.For(); + runner.Execute(Arg.Any()).Returns(new CommandResult("foo", 0)); + } + [Test] [Category(TestCategory.CompatibleOS.OnlyWindows)] public void DeterminesCorrectScriptTypePreferenceOrderWindows() @@ -56,8 +63,7 @@ public void ExecuteWithoutEnvironmentVarsPassesEmptyDictionaryToWrappers() var engine = new ScriptEngine(new List { capturingWrapper }, Substitute.For()); var variables = new CalamariVariables(); - var runner = Substitute.For(); - + engine.Execute(new Script("test.ps1"), variables, runner); receivedEnvironmentVars.Should().NotBeNull(); @@ -75,7 +81,6 @@ public void ExecuteWithEnvironmentVarsPassesDictionaryToWrappers() var engine = new ScriptEngine(new List { capturingWrapper }, Substitute.For()); var variables = new CalamariVariables(); - var runner = Substitute.For(); var envVars = new Dictionary { { "AWS_REGION", "us-east-1" } }; engine.Execute(new Script("test.ps1"), variables, runner, envVars);