From 15cb558d8f557ef114ab6976e76b2ca561df9422 Mon Sep 17 00:00:00 2001 From: eric sciple Date: Wed, 11 Feb 2026 09:44:01 -0600 Subject: [PATCH] Fix parser comparison mismatches (#4220) --- .../ActionManifestManagerWrapper.cs | 159 ++++- src/Runner.Worker/InternalsVisibleTo.cs | 3 + .../PipelineTemplateEvaluatorWrapper.cs | 95 ++- .../Conversion/WorkflowTemplateConverter.cs | 21 +- .../ActionManifestParserComparisonL0.cs | 424 ++++++++++++++ .../PipelineTemplateEvaluatorWrapperL0.cs | 550 ++++++++++++++++++ 6 files changed, 1237 insertions(+), 15 deletions(-) create mode 100644 src/Runner.Worker/InternalsVisibleTo.cs create mode 100644 src/Test/L0/Worker/ActionManifestParserComparisonL0.cs create mode 100644 src/Test/L0/Worker/PipelineTemplateEvaluatorWrapperL0.cs diff --git a/src/Runner.Worker/ActionManifestManagerWrapper.cs b/src/Runner.Worker/ActionManifestManagerWrapper.cs index aa265dbf4..6d893fd82 100644 --- a/src/Runner.Worker/ActionManifestManagerWrapper.cs +++ b/src/Runner.Worker/ActionManifestManagerWrapper.cs @@ -84,7 +84,8 @@ namespace GitHub.Runner.Worker "EvaluateContainerEnvironment", () => _legacyManager.EvaluateContainerEnvironment(executionContext, token, extraExpressionValues), () => _newManager.EvaluateContainerEnvironment(executionContext, ConvertToNewToken(token) as GitHub.Actions.WorkflowParser.ObjectTemplating.Tokens.MappingToken, ConvertToNewExpressionValues(extraExpressionValues)), - (legacyResult, newResult) => { + (legacyResult, newResult) => + { var trace = HostContext.GetTrace(nameof(ActionManifestManagerWrapper)); return CompareDictionaries(trace, legacyResult, newResult, "ContainerEnvironment"); }); @@ -165,9 +166,150 @@ namespace GitHub.Runner.Worker return null; } - // Serialize new steps and deserialize to old steps - var json = StringUtil.ConvertToJson(newSteps, Newtonsoft.Json.Formatting.None); - return StringUtil.ConvertFromJson>(json); + var result = new List(); + foreach (var step in newSteps) + { + var actionStep = new GitHub.DistributedTask.Pipelines.ActionStep + { + ContextName = step.Id, + }; + + if (step is GitHub.Actions.WorkflowParser.RunStep runStep) + { + actionStep.Condition = ExtractConditionString(runStep.If); + actionStep.DisplayNameToken = ConvertToLegacyToken(runStep.Name); + actionStep.ContinueOnError = ConvertToLegacyToken(runStep.ContinueOnError); + actionStep.TimeoutInMinutes = ConvertToLegacyToken(runStep.TimeoutMinutes); + actionStep.Environment = ConvertToLegacyToken(runStep.Env); + actionStep.Reference = new GitHub.DistributedTask.Pipelines.ScriptReference(); + actionStep.Inputs = BuildRunStepInputs(runStep); + } + else if (step is GitHub.Actions.WorkflowParser.ActionStep usesStep) + { + actionStep.Condition = ExtractConditionString(usesStep.If); + actionStep.DisplayNameToken = ConvertToLegacyToken(usesStep.Name); + actionStep.ContinueOnError = ConvertToLegacyToken(usesStep.ContinueOnError); + actionStep.TimeoutInMinutes = ConvertToLegacyToken(usesStep.TimeoutMinutes); + actionStep.Environment = ConvertToLegacyToken(usesStep.Env); + actionStep.Reference = ParseActionReference(usesStep.Uses?.Value); + actionStep.Inputs = ConvertToLegacyToken(usesStep.With); + } + + result.Add(actionStep); + } + return result; + } + + private string ExtractConditionString(GitHub.Actions.WorkflowParser.ObjectTemplating.Tokens.BasicExpressionToken ifToken) + { + if (ifToken == null) + { + return null; + } + + // The Expression property is internal, so we use ToString() which formats as "${{ expr }}" + // Then strip the delimiters to get just the expression + var str = ifToken.ToString(); + if (str.StartsWith("${{") && str.EndsWith("}}")) + { + return str.Substring(3, str.Length - 5).Trim(); + } + return str; + } + + private MappingToken BuildRunStepInputs(GitHub.Actions.WorkflowParser.RunStep runStep) + { + var inputs = new MappingToken(null, null, null); + + // script (from run) + if (runStep.Run != null) + { + inputs.Add( + new StringToken(null, null, null, "script"), + ConvertToLegacyToken(runStep.Run)); + } + + // shell + if (runStep.Shell != null) + { + inputs.Add( + new StringToken(null, null, null, "shell"), + ConvertToLegacyToken(runStep.Shell)); + } + + // working-directory + if (runStep.WorkingDirectory != null) + { + inputs.Add( + new StringToken(null, null, null, "workingDirectory"), + ConvertToLegacyToken(runStep.WorkingDirectory)); + } + + return inputs.Count > 0 ? inputs : null; + } + + private GitHub.DistributedTask.Pipelines.ActionStepDefinitionReference ParseActionReference(string uses) + { + if (string.IsNullOrEmpty(uses)) + { + return null; + } + + // Docker reference: docker://image:tag + if (uses.StartsWith("docker://", StringComparison.OrdinalIgnoreCase)) + { + return new GitHub.DistributedTask.Pipelines.ContainerRegistryReference + { + Image = uses.Substring("docker://".Length) + }; + } + + // Local path reference: ./path/to/action + if (uses.StartsWith("./") || uses.StartsWith(".\\")) + { + return new GitHub.DistributedTask.Pipelines.RepositoryPathReference + { + RepositoryType = "self", + Path = uses + }; + } + + // Repository reference: owner/repo@ref or owner/repo/path@ref + var atIndex = uses.LastIndexOf('@'); + string refPart = null; + string repoPart = uses; + + if (atIndex > 0) + { + refPart = uses.Substring(atIndex + 1); + repoPart = uses.Substring(0, atIndex); + } + + // Split by / to get owner/repo and optional path + var parts = repoPart.Split('/'); + string name; + string path = null; + + if (parts.Length >= 2) + { + name = $"{parts[0]}/{parts[1]}"; + if (parts.Length > 2) + { + path = string.Join("/", parts, 2, parts.Length - 2); + } + } + else + { + name = repoPart; + } + + return new GitHub.DistributedTask.Pipelines.RepositoryPathReference + { + RepositoryType = "GitHub", + Name = name, + Ref = refPart, + Path = path + }; } private T ConvertToLegacyToken(GitHub.Actions.WorkflowParser.ObjectTemplating.Tokens.TemplateToken newToken) where T : TemplateToken @@ -633,6 +775,14 @@ namespace GitHub.Runner.Worker return false; } + // Check for known equivalent error patterns (e.g., JSON parse errors) + // where both parsers correctly reject invalid input but with different wording + if (PipelineTemplateEvaluatorWrapper.HasJsonExceptionType(legacyException) && PipelineTemplateEvaluatorWrapper.HasJsonExceptionType(newException)) + { + trace.Info("CompareExceptions - both exceptions are JSON parse errors, treating as matched"); + return true; + } + // Compare exception messages recursively (including inner exceptions) var legacyMessages = GetExceptionMessages(legacyException); var newMessages = GetExceptionMessages(newException); @@ -697,5 +847,6 @@ namespace GitHub.Runner.Worker return messages; } + } } diff --git a/src/Runner.Worker/InternalsVisibleTo.cs b/src/Runner.Worker/InternalsVisibleTo.cs new file mode 100644 index 000000000..a825116a6 --- /dev/null +++ b/src/Runner.Worker/InternalsVisibleTo.cs @@ -0,0 +1,3 @@ +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("Test")] diff --git a/src/Runner.Worker/PipelineTemplateEvaluatorWrapper.cs b/src/Runner.Worker/PipelineTemplateEvaluatorWrapper.cs index 53742469b..61dfdacce 100644 --- a/src/Runner.Worker/PipelineTemplateEvaluatorWrapper.cs +++ b/src/Runner.Worker/PipelineTemplateEvaluatorWrapper.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using GitHub.Actions.WorkflowParser; using GitHub.DistributedTask.Expressions2; @@ -216,12 +216,15 @@ namespace GitHub.Runner.Worker } } - private TLegacy EvaluateAndCompare( + internal TLegacy EvaluateAndCompare( string methodName, Func legacyEvaluator, Func newEvaluator, Func resultComparer) { + // Capture cancellation state before evaluation + var cancellationRequestedBefore = _context.CancellationToken.IsCancellationRequested; + // Legacy evaluator var legacyException = default(Exception); var legacyResult = default(TLegacy); @@ -253,14 +256,18 @@ namespace GitHub.Runner.Worker newException = ex; } + // Capture cancellation state after evaluation + var cancellationRequestedAfter = _context.CancellationToken.IsCancellationRequested; + // Compare results or exceptions + bool hasMismatch = false; if (legacyException != null || newException != null) { // Either one or both threw exceptions - compare them if (!CompareExceptions(legacyException, newException)) { _trace.Info($"{methodName} exception mismatch"); - RecordMismatch($"{methodName}"); + hasMismatch = true; } } else @@ -269,6 +276,20 @@ namespace GitHub.Runner.Worker if (!resultComparer(legacyResult, newResult)) { _trace.Info($"{methodName} mismatch"); + hasMismatch = true; + } + } + + // Only record mismatch if it wasn't caused by a cancellation race condition + if (hasMismatch) + { + if (!cancellationRequestedBefore && cancellationRequestedAfter) + { + // Cancellation state changed during evaluation window - skip recording + _trace.Info($"{methodName} mismatch skipped due to cancellation race condition"); + } + else + { RecordMismatch($"{methodName}"); } } @@ -612,6 +633,13 @@ namespace GitHub.Runner.Worker return false; } + // Check for known equivalent error patterns (e.g., JSON parse errors) + // where both parsers correctly reject invalid input but with different wording + if (IsKnownEquivalentErrorPattern(legacyException, newException)) + { + return true; + } + // Compare exception messages recursively (including inner exceptions) var legacyMessages = GetExceptionMessages(legacyException); var newMessages = GetExceptionMessages(newException); @@ -634,6 +662,67 @@ namespace GitHub.Runner.Worker return true; } + /// + /// Checks if two exceptions match a known pattern where both parsers correctly reject + /// invalid input but with different error messages (e.g., JSON parse errors from fromJSON). + /// + private bool IsKnownEquivalentErrorPattern(Exception legacyException, Exception newException) + { + // fromJSON('') - both parsers fail when parsing empty string as JSON + // The error messages differ but both indicate JSON parsing failure. + // Legacy throws raw JsonReaderException: "Error reading JToken from JsonReader..." + // New wraps it: "Error parsing fromJson" with inner JsonReaderException + // Both may be wrapped in TemplateValidationException: "The template is not valid..." + if (HasJsonExceptionType(legacyException) && HasJsonExceptionType(newException)) + { + _trace.Info("CompareExceptions - both exceptions are JSON parse errors, treating as matched"); + return true; + } + + return false; + } + + /// + /// Checks if the exception chain contains a JSON-related exception type. + /// + internal static bool HasJsonExceptionType(Exception ex) + { + var toProcess = new Queue(); + toProcess.Enqueue(ex); + int count = 0; + + while (toProcess.Count > 0 && count < 50) + { + var current = toProcess.Dequeue(); + if (current == null) continue; + + count++; + + if (current is Newtonsoft.Json.JsonReaderException || + current is System.Text.Json.JsonException) + { + return true; + } + + if (current is AggregateException aggregateEx) + { + foreach (var innerEx in aggregateEx.InnerExceptions) + { + if (innerEx != null && count < 50) + { + toProcess.Enqueue(innerEx); + } + } + } + else if (current.InnerException != null) + { + toProcess.Enqueue(current.InnerException); + } + } + + return false; + } + private IList GetExceptionMessages(Exception ex) { var messages = new List(); diff --git a/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs b/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs index 0c7a74564..7c5764cb3 100644 --- a/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs +++ b/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs @@ -1,4 +1,4 @@ -#nullable disable // Consider removing in the future to minimize likelihood of NullReferenceException; refer https://learn.microsoft.com/en-us/dotnet/csharp/nullable-references +#nullable disable // Consider removing in the future to minimize likelihood of NullReferenceException; refer https://learn.microsoft.com/en-us/dotnet/csharp/nullable-references using System; using System.Collections.Generic; @@ -43,7 +43,7 @@ namespace GitHub.Actions.WorkflowParser.Conversion { case WorkflowTemplateConstants.On: var inputTypes = ConvertToOnWorkflowDispatchInputTypes(workflowPair.Value); - foreach(var item in inputTypes) + foreach (var item in inputTypes) { result.InputTypes.TryAdd(item.Key, item.Value); } @@ -432,7 +432,7 @@ namespace GitHub.Actions.WorkflowParser.Conversion context.Error(snapshotToken, $"job {WorkflowTemplateConstants.Snapshot} {WorkflowTemplateConstants.ImageName} is required."); return null; } - + return new Snapshot { ImageName = imageName, @@ -445,7 +445,7 @@ namespace GitHub.Actions.WorkflowParser.Conversion { var versionSegments = versionString.Split("."); - if (versionSegments.Length != 2 || + if (versionSegments.Length != 2 || !versionSegments[1].Equals("*") || !Int32.TryParse(versionSegments[0], NumberStyles.None, CultureInfo.InvariantCulture, result: out int parsedMajor) || parsedMajor < 0) @@ -1154,7 +1154,12 @@ namespace GitHub.Actions.WorkflowParser.Conversion if (String.IsNullOrEmpty(result.Image)) { - context.Error(value, "Container image cannot be empty"); + // Only error during early validation (parse time) + // At runtime (expression evaluation), empty image = no container + if (isEarlyValidation) + { + context.Error(value, "Container image cannot be empty"); + } return null; } @@ -1838,9 +1843,9 @@ namespace GitHub.Actions.WorkflowParser.Conversion case "actions": permissions.Actions = permissionLevel; break; - case "artifact-metadata": - permissions.ArtifactMetadata = permissionLevel; - break; + case "artifact-metadata": + permissions.ArtifactMetadata = permissionLevel; + break; case "attestations": permissions.Attestations = permissionLevel; break; diff --git a/src/Test/L0/Worker/ActionManifestParserComparisonL0.cs b/src/Test/L0/Worker/ActionManifestParserComparisonL0.cs new file mode 100644 index 000000000..d7039767c --- /dev/null +++ b/src/Test/L0/Worker/ActionManifestParserComparisonL0.cs @@ -0,0 +1,424 @@ +using GitHub.Actions.WorkflowParser; +using GitHub.DistributedTask.ObjectTemplating.Tokens; +using GitHub.DistributedTask.Pipelines.ContextData; +using GitHub.DistributedTask.Pipelines.ObjectTemplating; +using GitHub.DistributedTask.WebApi; +using GitHub.Runner.Common; +using GitHub.Runner.Sdk; +using GitHub.Runner.Worker; +using LegacyContextData = GitHub.DistributedTask.Pipelines.ContextData; +using LegacyExpressions = GitHub.DistributedTask.Expressions2; +using Moq; +using Newtonsoft.Json; +using System; +using System.Collections.Generic; +using System.IO; +using System.Runtime.CompilerServices; +using System.Threading; +using Xunit; + +namespace GitHub.Runner.Common.Tests.Worker +{ + /// + /// Tests for parser comparison wrapper classes. + /// + public sealed class ActionManifestParserComparisonL0 + { + private CancellationTokenSource _ecTokenSource; + private Mock _ec; + private TestHostContext _hc; + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void ConvertToLegacySteps_ProducesCorrectSteps_WithExplicitPropertyMapping() + { + try + { + // Arrange - Test that ActionManifestManagerWrapper properly converts new steps to legacy format + Setup(); + + // Enable comparison feature + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + // Register required services + var legacyManager = new ActionManifestManagerLegacy(); + legacyManager.Initialize(_hc); + _hc.SetSingleton(legacyManager); + + var newManager = new ActionManifestManager(); + newManager.Initialize(_hc); + _hc.SetSingleton(newManager); + + var wrapper = new ActionManifestManagerWrapper(); + wrapper.Initialize(_hc); + + var manifestPath = Path.Combine(TestUtil.GetTestDataPath(), "conditional_composite_action.yml"); + + // Act - Load through the wrapper (which internally converts) + var result = wrapper.Load(_ec.Object, manifestPath); + + // Assert + Assert.NotNull(result); + Assert.Equal(ActionExecutionType.Composite, result.Execution.ExecutionType); + + var compositeExecution = result.Execution as CompositeActionExecutionData; + Assert.NotNull(compositeExecution); + Assert.NotNull(compositeExecution.Steps); + Assert.Equal(6, compositeExecution.Steps.Count); + + // Verify steps are NOT null (this was the bug - JSON round-trip produced nulls) + foreach (var step in compositeExecution.Steps) + { + Assert.NotNull(step); + Assert.NotNull(step.Reference); + Assert.IsType(step.Reference); + } + + // Verify step with condition + var successStep = compositeExecution.Steps[2]; + Assert.Equal("success-conditional", successStep.ContextName); + Assert.Equal("success()", successStep.Condition); + + // Verify step with complex condition + var lastStep = compositeExecution.Steps[5]; + Assert.Contains("inputs.exit-code == 1", lastStep.Condition); + Assert.Contains("failure()", lastStep.Condition); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateJobContainer_EmptyImage_BothParsersReturnNull() + { + try + { + // Arrange - Test that both parsers return null for empty container image at runtime + Setup(); + + var fileTable = new List(); + + // Create legacy evaluator + var legacyTraceWriter = new GitHub.DistributedTask.ObjectTemplating.EmptyTraceWriter(); + var schema = PipelineTemplateSchemaFactory.GetSchema(); + var legacyEvaluator = new PipelineTemplateEvaluator(legacyTraceWriter, schema, fileTable); + + // Create new evaluator + var newTraceWriter = new GitHub.Actions.WorkflowParser.ObjectTemplating.EmptyTraceWriter(); + var newEvaluator = new WorkflowTemplateEvaluator(newTraceWriter, fileTable, features: null); + + // Create a token representing an empty container image (simulates expression evaluated to empty string) + var emptyImageToken = new StringToken(null, null, null, ""); + + var contextData = new DictionaryContextData(); + var expressionFunctions = new List(); + + // Act - Call both evaluators + var legacyResult = legacyEvaluator.EvaluateJobContainer(emptyImageToken, contextData, expressionFunctions); + + // Convert token for new evaluator + var newToken = new GitHub.Actions.WorkflowParser.ObjectTemplating.Tokens.StringToken(null, null, null, ""); + var newContextData = new GitHub.Actions.Expressions.Data.DictionaryExpressionData(); + var newExpressionFunctions = new List(); + + var newResult = newEvaluator.EvaluateJobContainer(newToken, newContextData, newExpressionFunctions); + + // Assert - Both should return null for empty image (no container) + Assert.Null(legacyResult); + Assert.Null(newResult); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void FromJsonEmptyString_BothParsersFail_WithDifferentMessages() + { + // This test verifies that both parsers fail with different error messages when parsing fromJSON('') + // The comparison layer should treat these as semantically equivalent (both are JSON parse errors) + try + { + Setup(); + + var fileTable = new List(); + + // Create legacy evaluator + var legacyTraceWriter = new GitHub.DistributedTask.ObjectTemplating.EmptyTraceWriter(); + var schema = PipelineTemplateSchemaFactory.GetSchema(); + var legacyEvaluator = new PipelineTemplateEvaluator(legacyTraceWriter, schema, fileTable); + + // Create new evaluator + var newTraceWriter = new GitHub.Actions.WorkflowParser.ObjectTemplating.EmptyTraceWriter(); + var newEvaluator = new WorkflowTemplateEvaluator(newTraceWriter, fileTable, features: null); + + // Create expression token for fromJSON('') + var legacyToken = new BasicExpressionToken(null, null, null, "fromJson('')"); + var newToken = new GitHub.Actions.WorkflowParser.ObjectTemplating.Tokens.BasicExpressionToken(null, null, null, "fromJson('')"); + + var contextData = new DictionaryContextData(); + var newContextData = new GitHub.Actions.Expressions.Data.DictionaryExpressionData(); + var expressionFunctions = new List(); + var newExpressionFunctions = new List(); + + // Act - Both should throw + Exception legacyException = null; + Exception newException = null; + + try + { + legacyEvaluator.EvaluateStepDisplayName(legacyToken, contextData, expressionFunctions); + } + catch (Exception ex) + { + legacyException = ex; + } + + try + { + newEvaluator.EvaluateStepName(newToken, newContextData, newExpressionFunctions); + } + catch (Exception ex) + { + newException = ex; + } + + // Assert - Both threw exceptions + Assert.NotNull(legacyException); + Assert.NotNull(newException); + + // Verify the error messages are different (which is why we need semantic comparison) + Assert.NotEqual(legacyException.Message, newException.Message); + + // Verify both are JSON parse errors (contain JSON-related error indicators) + var legacyFullMsg = GetFullExceptionMessage(legacyException); + var newFullMsg = GetFullExceptionMessage(newException); + + // At least one should contain indicators of JSON parsing failure + var legacyIsJsonError = legacyFullMsg.Contains("JToken") || + legacyFullMsg.Contains("JsonReader") || + legacyFullMsg.Contains("fromJson"); + var newIsJsonError = newFullMsg.Contains("JToken") || + newFullMsg.Contains("JsonReader") || + newFullMsg.Contains("fromJson"); + + Assert.True(legacyIsJsonError, $"Legacy exception should be JSON error: {legacyFullMsg}"); + Assert.True(newIsJsonError, $"New exception should be JSON error: {newFullMsg}"); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateDefaultInput_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var legacyManager = new ActionManifestManagerLegacy(); + legacyManager.Initialize(_hc); + _hc.SetSingleton(legacyManager); + + var newManager = new ActionManifestManager(); + newManager.Initialize(_hc); + _hc.SetSingleton(newManager); + + var wrapper = new ActionManifestManagerWrapper(); + wrapper.Initialize(_hc); + + _ec.Object.ExpressionValues["github"] = new LegacyContextData.DictionaryContextData(); + _ec.Object.ExpressionValues["strategy"] = new LegacyContextData.DictionaryContextData(); + _ec.Object.ExpressionValues["matrix"] = new LegacyContextData.DictionaryContextData(); + _ec.Object.ExpressionValues["steps"] = new LegacyContextData.DictionaryContextData(); + _ec.Object.ExpressionValues["job"] = new LegacyContextData.DictionaryContextData(); + _ec.Object.ExpressionValues["runner"] = new LegacyContextData.DictionaryContextData(); + _ec.Object.ExpressionValues["env"] = new LegacyContextData.DictionaryContextData(); + _ec.Object.ExpressionFunctions.Add(new LegacyExpressions.FunctionInfo("hashFiles", 1, 255)); + + var result = wrapper.EvaluateDefaultInput(_ec.Object, "testInput", new StringToken(null, null, null, "defaultValue")); + + Assert.Equal("defaultValue", result); + Assert.False(_ec.Object.Global.HasActionManifestMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateContainerArguments_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var legacyManager = new ActionManifestManagerLegacy(); + legacyManager.Initialize(_hc); + _hc.SetSingleton(legacyManager); + + var newManager = new ActionManifestManager(); + newManager.Initialize(_hc); + _hc.SetSingleton(newManager); + + var wrapper = new ActionManifestManagerWrapper(); + wrapper.Initialize(_hc); + + var arguments = new SequenceToken(null, null, null); + arguments.Add(new StringToken(null, null, null, "arg1")); + arguments.Add(new StringToken(null, null, null, "arg2")); + + var evaluateContext = new Dictionary(StringComparer.OrdinalIgnoreCase); + + var result = wrapper.EvaluateContainerArguments(_ec.Object, arguments, evaluateContext); + + Assert.Equal(2, result.Count); + Assert.Equal("arg1", result[0]); + Assert.Equal("arg2", result[1]); + Assert.False(_ec.Object.Global.HasActionManifestMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateContainerEnvironment_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var legacyManager = new ActionManifestManagerLegacy(); + legacyManager.Initialize(_hc); + _hc.SetSingleton(legacyManager); + + var newManager = new ActionManifestManager(); + newManager.Initialize(_hc); + _hc.SetSingleton(newManager); + + var wrapper = new ActionManifestManagerWrapper(); + wrapper.Initialize(_hc); + + var environment = new MappingToken(null, null, null); + environment.Add(new StringToken(null, null, null, "hello"), new StringToken(null, null, null, "world")); + + var evaluateContext = new Dictionary(StringComparer.OrdinalIgnoreCase); + + var result = wrapper.EvaluateContainerEnvironment(_ec.Object, environment, evaluateContext); + + Assert.Equal(1, result.Count); + Assert.Equal("world", result["hello"]); + Assert.False(_ec.Object.Global.HasActionManifestMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateCompositeOutputs_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var legacyManager = new ActionManifestManagerLegacy(); + legacyManager.Initialize(_hc); + _hc.SetSingleton(legacyManager); + + var newManager = new ActionManifestManager(); + newManager.Initialize(_hc); + _hc.SetSingleton(newManager); + + var wrapper = new ActionManifestManagerWrapper(); + wrapper.Initialize(_hc); + + var outputDef = new MappingToken(null, null, null); + outputDef.Add(new StringToken(null, null, null, "description"), new StringToken(null, null, null, "test output")); + outputDef.Add(new StringToken(null, null, null, "value"), new StringToken(null, null, null, "value1")); + + var token = new MappingToken(null, null, null); + token.Add(new StringToken(null, null, null, "output1"), outputDef); + + var evaluateContext = new Dictionary(StringComparer.OrdinalIgnoreCase); + + var result = wrapper.EvaluateCompositeOutputs(_ec.Object, token, evaluateContext); + + Assert.NotNull(result); + Assert.False(_ec.Object.Global.HasActionManifestMismatch); + } + finally + { + Teardown(); + } + } + + private string GetFullExceptionMessage(Exception ex) + { + var messages = new List(); + var current = ex; + while (current != null) + { + messages.Add(current.Message); + current = current.InnerException; + } + return string.Join(" -> ", messages); + } + + private void Setup([CallerMemberName] string name = "") + { + _ecTokenSource?.Dispose(); + _ecTokenSource = new CancellationTokenSource(); + + _hc = new TestHostContext(this, name); + + var expressionValues = new LegacyContextData.DictionaryContextData(); + var expressionFunctions = new List(); + + _ec = new Mock(); + _ec.Setup(x => x.Global) + .Returns(new GlobalContext + { + FileTable = new List(), + Variables = new Variables(_hc, new Dictionary()), + WriteDebug = true, + }); + _ec.Setup(x => x.CancellationToken).Returns(_ecTokenSource.Token); + _ec.Setup(x => x.ExpressionValues).Returns(expressionValues); + _ec.Setup(x => x.ExpressionFunctions).Returns(expressionFunctions); + _ec.Setup(x => x.Write(It.IsAny(), It.IsAny())).Callback((string tag, string message) => { _hc.GetTrace().Info($"{tag}{message}"); }); + _ec.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())).Callback((Issue issue, ExecutionContextLogOptions logOptions) => { _hc.GetTrace().Info($"[{issue.Type}]{logOptions.LogMessageOverride ?? issue.Message}"); }); + } + + private void Teardown() + { + _hc?.Dispose(); + } + } +} diff --git a/src/Test/L0/Worker/PipelineTemplateEvaluatorWrapperL0.cs b/src/Test/L0/Worker/PipelineTemplateEvaluatorWrapperL0.cs new file mode 100644 index 000000000..2caf5dba0 --- /dev/null +++ b/src/Test/L0/Worker/PipelineTemplateEvaluatorWrapperL0.cs @@ -0,0 +1,550 @@ +using GitHub.DistributedTask.ObjectTemplating.Tokens; +using GitHub.DistributedTask.Pipelines.ContextData; +using GitHub.DistributedTask.Pipelines.ObjectTemplating; +using GitHub.DistributedTask.WebApi; +using GitHub.Runner.Common; +using GitHub.Runner.Sdk; +using GitHub.Runner.Worker; +using LegacyContextData = GitHub.DistributedTask.Pipelines.ContextData; +using LegacyExpressions = GitHub.DistributedTask.Expressions2; +using Moq; +using System; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using System.Threading; +using Xunit; + +namespace GitHub.Runner.Common.Tests.Worker +{ + public sealed class PipelineTemplateEvaluatorWrapperL0 + { + private CancellationTokenSource _ecTokenSource; + private Mock _ec; + private TestHostContext _hc; + + // ------------------------------------------------------------------- + // EvaluateAndCompare core behavior + // ------------------------------------------------------------------- + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateAndCompare_DoesNotRecordMismatch_WhenResultsMatch() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + + var token = new StringToken(null, null, null, "test-value"); + var contextData = new DictionaryContextData(); + var expressionFunctions = new List(); + + var result = wrapper.EvaluateStepDisplayName(token, contextData, expressionFunctions); + + Assert.Equal("test-value", result); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateAndCompare_SkipsMismatchRecording_WhenCancellationOccursDuringEvaluation() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + + // Call EvaluateAndCompare directly: the new evaluator cancels the token + // and returns a different value, forcing hasMismatch = true. + // Because cancellation flipped during the evaluation window, the + // mismatch should be skipped. + var result = wrapper.EvaluateAndCompare( + "TestCancellationSkip", + () => "legacy-value", + () => + { + _ecTokenSource.Cancel(); + return "different-value"; + }, + (legacy, @new) => string.Equals(legacy, @new, StringComparison.Ordinal)); + + Assert.Equal("legacy-value", result); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateAndCompare_RecordsMismatch_WhenResultsDifferWithoutCancellation() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + + // Different results without cancellation — mismatch SHOULD be recorded. + var result = wrapper.EvaluateAndCompare( + "TestMismatchRecorded", + () => "legacy-value", + () => "different-value", + (legacy, @new) => string.Equals(legacy, @new, StringComparison.Ordinal)); + + Assert.Equal("legacy-value", result); + Assert.True(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + // ------------------------------------------------------------------- + // Smoke tests — both parsers agree, no mismatch recorded + // ------------------------------------------------------------------- + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateStepContinueOnError_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + var token = new BooleanToken(null, null, null, true); + var contextData = new DictionaryContextData(); + var functions = new List(); + + var result = wrapper.EvaluateStepContinueOnError(token, contextData, functions); + + Assert.True(result); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateStepEnvironment_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + var token = new MappingToken(null, null, null); + token.Add(new StringToken(null, null, null, "FOO"), new StringToken(null, null, null, "bar")); + var contextData = new DictionaryContextData(); + var functions = new List(); + + var result = wrapper.EvaluateStepEnvironment(token, contextData, functions, StringComparer.OrdinalIgnoreCase); + + Assert.NotNull(result); + Assert.Equal("bar", result["FOO"]); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateStepIf_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + var token = new BasicExpressionToken(null, null, null, "true"); + var contextData = new DictionaryContextData(); + var functions = new List(); + var expressionState = new List>(); + + var result = wrapper.EvaluateStepIf(token, contextData, functions, expressionState); + + Assert.True(result); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateStepInputs_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + var token = new MappingToken(null, null, null); + token.Add(new StringToken(null, null, null, "input1"), new StringToken(null, null, null, "val1")); + var contextData = new DictionaryContextData(); + var functions = new List(); + + var result = wrapper.EvaluateStepInputs(token, contextData, functions); + + Assert.NotNull(result); + Assert.Equal("val1", result["input1"]); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateStepTimeout_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + var token = new NumberToken(null, null, null, 10); + var contextData = new DictionaryContextData(); + var functions = new List(); + + var result = wrapper.EvaluateStepTimeout(token, contextData, functions); + + Assert.Equal(10, result); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateJobContainer_EmptyImage_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + var token = new StringToken(null, null, null, ""); + var contextData = new DictionaryContextData(); + var functions = new List(); + + var result = wrapper.EvaluateJobContainer(token, contextData, functions); + + Assert.Null(result); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateJobOutput_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + var token = new MappingToken(null, null, null); + token.Add(new StringToken(null, null, null, "out1"), new StringToken(null, null, null, "val1")); + var contextData = new DictionaryContextData(); + var functions = new List(); + + var result = wrapper.EvaluateJobOutput(token, contextData, functions); + + Assert.NotNull(result); + Assert.Equal("val1", result["out1"]); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateEnvironmentUrl_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + var token = new StringToken(null, null, null, "https://example.com"); + var contextData = new DictionaryContextData(); + var functions = new List(); + + var result = wrapper.EvaluateEnvironmentUrl(token, contextData, functions); + + Assert.NotNull(result); + var stringResult = result as StringToken; + Assert.NotNull(stringResult); + Assert.Equal("https://example.com", stringResult.Value); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateJobDefaultsRun_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + var token = new MappingToken(null, null, null); + token.Add(new StringToken(null, null, null, "shell"), new StringToken(null, null, null, "bash")); + var contextData = new DictionaryContextData(); + var functions = new List(); + + var result = wrapper.EvaluateJobDefaultsRun(token, contextData, functions); + + Assert.NotNull(result); + Assert.Equal("bash", result["shell"]); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateJobServiceContainers_Null_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + var contextData = new DictionaryContextData(); + var functions = new List(); + + var result = wrapper.EvaluateJobServiceContainers(null, contextData, functions); + + Assert.Null(result); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateJobSnapshotRequest_Null_BothParsersAgree() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + var contextData = new DictionaryContextData(); + var functions = new List(); + + var result = wrapper.EvaluateJobSnapshotRequest(null, contextData, functions); + + Assert.Null(result); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + // ------------------------------------------------------------------- + // JSON parse error equivalence via EvaluateAndCompare + // ------------------------------------------------------------------- + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateAndCompare_JsonReaderExceptions_TreatedAsEquivalent() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + + // Both throw JsonReaderException with different messages — should be treated as equivalent + var legacyEx = new Newtonsoft.Json.JsonReaderException("Error reading JToken from JsonReader. Path '', line 0, position 0."); + var newEx = new Newtonsoft.Json.JsonReaderException("Error parsing fromJson", new Newtonsoft.Json.JsonReaderException("Unexpected end")); + + Assert.Throws(() => + wrapper.EvaluateAndCompare( + "TestJsonEquivalence", + () => throw legacyEx, + () => throw newEx, + (a, b) => string.Equals(a, b, StringComparison.Ordinal))); + + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateAndCompare_MixedJsonExceptionTypes_TreatedAsEquivalent() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + + // Legacy throws Newtonsoft JsonReaderException, new throws System.Text.Json.JsonException + var legacyEx = new Newtonsoft.Json.JsonReaderException("Error reading JToken"); + var newEx = new System.Text.Json.JsonException("Error parsing fromJson"); + + Assert.Throws(() => + wrapper.EvaluateAndCompare( + "TestMixedJsonTypes", + () => throw legacyEx, + () => throw newEx, + (a, b) => string.Equals(a, b, StringComparison.Ordinal))); + + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateAndCompare_NonJsonExceptions_RecordsMismatch() + { + try + { + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + + // Both throw non-JSON exceptions with different messages — should record mismatch + var legacyEx = new InvalidOperationException("some error"); + var newEx = new InvalidOperationException("different error"); + + Assert.Throws(() => + wrapper.EvaluateAndCompare( + "TestNonJsonMismatch", + () => throw legacyEx, + () => throw newEx, + (a, b) => string.Equals(a, b, StringComparison.Ordinal))); + + Assert.True(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + // ------------------------------------------------------------------- + // Helpers + // ------------------------------------------------------------------- + + private void Setup([CallerMemberName] string name = "") + { + _ecTokenSource?.Dispose(); + _ecTokenSource = new CancellationTokenSource(); + + _hc = new TestHostContext(this, name); + + var expressionValues = new LegacyContextData.DictionaryContextData(); + var expressionFunctions = new List(); + + _ec = new Mock(); + _ec.Setup(x => x.Global) + .Returns(new GlobalContext + { + FileTable = new List(), + Variables = new Variables(_hc, new Dictionary()), + WriteDebug = true, + }); + _ec.Setup(x => x.CancellationToken).Returns(_ecTokenSource.Token); + _ec.Setup(x => x.ExpressionValues).Returns(expressionValues); + _ec.Setup(x => x.ExpressionFunctions).Returns(expressionFunctions); + _ec.Setup(x => x.Write(It.IsAny(), It.IsAny())).Callback((string tag, string message) => { _hc.GetTrace().Info($"{tag}{message}"); }); + _ec.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())).Callback((Issue issue, ExecutionContextLogOptions logOptions) => { _hc.GetTrace().Info($"[{issue.Type}]{logOptions.LogMessageOverride ?? issue.Message}"); }); + } + + private void Teardown() + { + _hc?.Dispose(); + } + } +}