From 9d9c03a688d87d7330dfb8c78638342eb8396d57 Mon Sep 17 00:00:00 2001 From: eric sciple Date: Tue, 3 Feb 2026 07:01:38 +0000 Subject: [PATCH 1/2] Fix parser comparison mismatches Fixes telemetry noise from parser comparison mismatches between legacy and new workflow parsers. ## Fixes - **ActionManifestMismatch: Load** - Fix `ConvertToLegacySteps()` to use explicit property mapping instead of broken JSON round-trip - **EvaluateJobContainer / EvaluateJobServiceContainers** - Only emit error for empty container image during early validation, not at runtime - **TemplateEvaluatorMismatch (fromJSON)** - Treat JSON parse errors as matching even when error messages differ - **Cancellation race conditions** - Skip mismatch recording when cancellation state changes during the evaluation window --- .../ActionManifestManagerWrapper.cs | 150 +++++++- src/Runner.Worker/InternalsVisibleTo.cs | 3 + .../PipelineTemplateEvaluatorWrapper.cs | 67 +++- .../Conversion/WorkflowTemplateConverter.cs | 21 +- .../ActionManifestParserComparisonL0.cs | 343 ++++++++++++++++++ 5 files changed, 570 insertions(+), 14 deletions(-) create mode 100644 src/Runner.Worker/InternalsVisibleTo.cs create mode 100644 src/Test/L0/Worker/ActionManifestParserComparisonL0.cs diff --git a/src/Runner.Worker/ActionManifestManagerWrapper.cs b/src/Runner.Worker/ActionManifestManagerWrapper.cs index aa265dbf4b4..974eac7dcb7 100644 --- a/src/Runner.Worker/ActionManifestManagerWrapper.cs +++ b/src/Runner.Worker/ActionManifestManagerWrapper.cs @@ -84,7 +84,8 @@ public Dictionary EvaluateContainerEnvironment( "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 @@ private ActionExecutionData ConvertToLegacyExecution(ActionExecutionData executi 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 diff --git a/src/Runner.Worker/InternalsVisibleTo.cs b/src/Runner.Worker/InternalsVisibleTo.cs new file mode 100644 index 00000000000..a825116a601 --- /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 53742469bd0..ae7ebc1c8f8 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; @@ -222,6 +222,9 @@ private TLegacy EvaluateAndCompare( 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 @@ private TLegacy EvaluateAndCompare( 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 @@ private TLegacy EvaluateAndCompare( 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 @@ private bool CompareExceptions(Exception legacyException, Exception newException 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,41 @@ private bool CompareExceptions(Exception legacyException, Exception newException 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) + { + // Get all messages in the exception chain + var legacyMessages = string.Join(" | ", GetExceptionMessages(legacyException)); + var newMessages = string.Join(" | ", GetExceptionMessages(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 (IsJsonParseError(legacyMessages) && IsJsonParseError(newMessages)) + { + _trace.Info("CompareExceptions - both exceptions are JSON parse errors (semantically equivalent)"); + return true; + } + + return false; + } + + /// + /// Checks if the exception message chain indicates a JSON parsing error. + /// + private bool IsJsonParseError(string messages) + { + // Common patterns for JSON parse errors from fromJSON function + return messages.Contains("Error reading JToken from JsonReader") || + messages.Contains("Error parsing fromJson") || + messages.Contains("JsonReaderException"); + } + 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 0c7a74564cb..7c5764cb3ee 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 @@ internal static WorkflowTemplate ConvertToWorkflow( { 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 @@ internal static Snapshot ConvertToSnapshot( context.Error(snapshotToken, $"job {WorkflowTemplateConstants.Snapshot} {WorkflowTemplateConstants.ImageName} is required."); return null; } - + return new Snapshot { ImageName = imageName, @@ -445,7 +445,7 @@ private static bool IsSnapshotImageVersionValid(string versionString) { 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 @@ internal static JobContainer ConvertToJobContainer( 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 @@ private static Permissions ConvertToPermissions(TemplateContext context, Templat 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 00000000000..26231d7cc5b --- /dev/null +++ b/src/Test/L0/Worker/ActionManifestParserComparisonL0.cs @@ -0,0 +1,343 @@ +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 EvaluateAndCompare_SkipsMismatchRecording_WhenCancellationOccursDuringEvaluation() + { + try + { + // Arrange - Test that mismatches are not recorded when cancellation state changes during evaluation + Setup(); + + // Enable comparison feature + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + // Create the wrapper + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + + // Create a simple token for evaluation + var token = new StringToken(null, null, null, "test-value"); + var contextData = new DictionaryContextData(); + var expressionFunctions = new List(); + + // First evaluation without cancellation - should work normally + var result1 = wrapper.EvaluateStepDisplayName(token, contextData, expressionFunctions); + Assert.Equal("test-value", result1); + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + + // Now simulate a scenario where cancellation occurs during evaluation + // Cancel the token before next evaluation + _ecTokenSource.Cancel(); + + // Evaluate again - even if there were a mismatch, it should be skipped due to cancellation + var result2 = wrapper.EvaluateStepDisplayName(token, contextData, expressionFunctions); + Assert.Equal("test-value", result2); + + // Verify no mismatch was recorded (cancellation race detection should have prevented it) + // Note: In this test, both parsers return the same result, so there's no actual mismatch. + // The cancellation race detection is a safeguard for when results differ due to timing. + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void EvaluateAndCompare_DoesNotRecordMismatch_WhenResultsMatch() + { + try + { + // Arrange - Test that no mismatch is recorded when both parsers return matching results + Setup(); + + // Enable comparison feature + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + // Create the wrapper + var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + + // Create a simple token for evaluation + var token = new StringToken(null, null, null, "test-value"); + var contextData = new DictionaryContextData(); + var expressionFunctions = new List(); + + // Evaluation without cancellation - should work normally and not record mismatch for matching results + var result = wrapper.EvaluateStepDisplayName(token, contextData, expressionFunctions); + Assert.Equal("test-value", result); + + // Since both parsers return the same result, no mismatch should be recorded + Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + } + 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(); + } + } +} From d5a185319e1497c67651ec43308a76dd1c5814ed Mon Sep 17 00:00:00 2001 From: eric sciple Date: Tue, 10 Feb 2026 18:23:00 +0000 Subject: [PATCH 2/2] Fix cancellation race-skip test, harden JSON error matching, add wrapper test coverage --- .../ActionManifestManagerWrapper.cs | 9 + .../PipelineTemplateEvaluatorWrapper.cs | 52 +- .../ActionManifestParserComparisonL0.cs | 155 +++-- .../PipelineTemplateEvaluatorWrapperL0.cs | 550 ++++++++++++++++++ 4 files changed, 716 insertions(+), 50 deletions(-) create mode 100644 src/Test/L0/Worker/PipelineTemplateEvaluatorWrapperL0.cs diff --git a/src/Runner.Worker/ActionManifestManagerWrapper.cs b/src/Runner.Worker/ActionManifestManagerWrapper.cs index 974eac7dcb7..6d893fd8252 100644 --- a/src/Runner.Worker/ActionManifestManagerWrapper.cs +++ b/src/Runner.Worker/ActionManifestManagerWrapper.cs @@ -775,6 +775,14 @@ private bool CompareExceptions(Tracing trace, Exception legacyException, Excepti 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); @@ -839,5 +847,6 @@ private IList GetExceptionMessages(Exception ex) return messages; } + } } diff --git a/src/Runner.Worker/PipelineTemplateEvaluatorWrapper.cs b/src/Runner.Worker/PipelineTemplateEvaluatorWrapper.cs index ae7ebc1c8f8..61dfdacce6d 100644 --- a/src/Runner.Worker/PipelineTemplateEvaluatorWrapper.cs +++ b/src/Runner.Worker/PipelineTemplateEvaluatorWrapper.cs @@ -216,7 +216,7 @@ private void RecordComparisonError(string errorDetails) } } - private TLegacy EvaluateAndCompare( + internal TLegacy EvaluateAndCompare( string methodName, Func legacyEvaluator, Func newEvaluator, @@ -668,18 +668,14 @@ private bool CompareExceptions(Exception legacyException, Exception newException /// private bool IsKnownEquivalentErrorPattern(Exception legacyException, Exception newException) { - // Get all messages in the exception chain - var legacyMessages = string.Join(" | ", GetExceptionMessages(legacyException)); - var newMessages = string.Join(" | ", GetExceptionMessages(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 (IsJsonParseError(legacyMessages) && IsJsonParseError(newMessages)) + if (HasJsonExceptionType(legacyException) && HasJsonExceptionType(newException)) { - _trace.Info("CompareExceptions - both exceptions are JSON parse errors (semantically equivalent)"); + _trace.Info("CompareExceptions - both exceptions are JSON parse errors, treating as matched"); return true; } @@ -687,14 +683,44 @@ private bool IsKnownEquivalentErrorPattern(Exception legacyException, Exception } /// - /// Checks if the exception message chain indicates a JSON parsing error. + /// Checks if the exception chain contains a JSON-related exception type. /// - private bool IsJsonParseError(string messages) + internal static bool HasJsonExceptionType(Exception ex) { - // Common patterns for JSON parse errors from fromJSON function - return messages.Contains("Error reading JToken from JsonReader") || - messages.Contains("Error parsing fromJson") || - messages.Contains("JsonReaderException"); + 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) diff --git a/src/Test/L0/Worker/ActionManifestParserComparisonL0.cs b/src/Test/L0/Worker/ActionManifestParserComparisonL0.cs index 26231d7cc5b..d7039767cde 100644 --- a/src/Test/L0/Worker/ActionManifestParserComparisonL0.cs +++ b/src/Test/L0/Worker/ActionManifestParserComparisonL0.cs @@ -222,41 +222,77 @@ public void FromJsonEmptyString_BothParsersFail_WithDifferentMessages() [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] - public void EvaluateAndCompare_SkipsMismatchRecording_WhenCancellationOccursDuringEvaluation() + public void EvaluateDefaultInput_BothParsersAgree() { try { - // Arrange - Test that mismatches are not recorded when cancellation state changes during evaluation Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); - // Enable comparison feature + 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"); - // Create the wrapper - var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + var legacyManager = new ActionManifestManagerLegacy(); + legacyManager.Initialize(_hc); + _hc.SetSingleton(legacyManager); - // Create a simple token for evaluation - var token = new StringToken(null, null, null, "test-value"); - var contextData = new DictionaryContextData(); - var expressionFunctions = new List(); + var newManager = new ActionManifestManager(); + newManager.Initialize(_hc); + _hc.SetSingleton(newManager); + + var wrapper = new ActionManifestManagerWrapper(); + wrapper.Initialize(_hc); - // First evaluation without cancellation - should work normally - var result1 = wrapper.EvaluateStepDisplayName(token, contextData, expressionFunctions); - Assert.Equal("test-value", result1); - Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + var arguments = new SequenceToken(null, null, null); + arguments.Add(new StringToken(null, null, null, "arg1")); + arguments.Add(new StringToken(null, null, null, "arg2")); - // Now simulate a scenario where cancellation occurs during evaluation - // Cancel the token before next evaluation - _ecTokenSource.Cancel(); + var evaluateContext = new Dictionary(StringComparer.OrdinalIgnoreCase); - // Evaluate again - even if there were a mismatch, it should be skipped due to cancellation - var result2 = wrapper.EvaluateStepDisplayName(token, contextData, expressionFunctions); - Assert.Equal("test-value", result2); + var result = wrapper.EvaluateContainerArguments(_ec.Object, arguments, evaluateContext); - // Verify no mismatch was recorded (cancellation race detection should have prevented it) - // Note: In this test, both parsers return the same result, so there's no actual mismatch. - // The cancellation race detection is a safeguard for when results differ due to timing. - Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + Assert.Equal(2, result.Count); + Assert.Equal("arg1", result[0]); + Assert.Equal("arg2", result[1]); + Assert.False(_ec.Object.Global.HasActionManifestMismatch); } finally { @@ -267,30 +303,75 @@ public void EvaluateAndCompare_SkipsMismatchRecording_WhenCancellationOccursDuri [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] - public void EvaluateAndCompare_DoesNotRecordMismatch_WhenResultsMatch() + public void EvaluateContainerEnvironment_BothParsersAgree() { try { - // Arrange - Test that no mismatch is recorded when both parsers return matching results Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); - // Enable comparison feature + 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"); - // Create the wrapper - var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object); + var legacyManager = new ActionManifestManagerLegacy(); + legacyManager.Initialize(_hc); + _hc.SetSingleton(legacyManager); - // Create a simple token for evaluation - var token = new StringToken(null, null, null, "test-value"); - var contextData = new DictionaryContextData(); - var expressionFunctions = new List(); + 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); - // Evaluation without cancellation - should work normally and not record mismatch for matching results - var result = wrapper.EvaluateStepDisplayName(token, contextData, expressionFunctions); - Assert.Equal("test-value", result); + var evaluateContext = new Dictionary(StringComparer.OrdinalIgnoreCase); - // Since both parsers return the same result, no mismatch should be recorded - Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch); + var result = wrapper.EvaluateCompositeOutputs(_ec.Object, token, evaluateContext); + + Assert.NotNull(result); + Assert.False(_ec.Object.Global.HasActionManifestMismatch); } finally { diff --git a/src/Test/L0/Worker/PipelineTemplateEvaluatorWrapperL0.cs b/src/Test/L0/Worker/PipelineTemplateEvaluatorWrapperL0.cs new file mode 100644 index 00000000000..2caf5dba019 --- /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(); + } + } +}