diff --git a/src/Runner.Worker/ActionRunner.cs b/src/Runner.Worker/ActionRunner.cs
index 257d60109..62fbbb812 100644
--- a/src/Runner.Worker/ActionRunner.cs
+++ b/src/Runner.Worker/ActionRunner.cs
@@ -25,7 +25,6 @@ namespace GitHub.Runner.Worker
public interface IActionRunner : IStep, IRunnerService
{
ActionRunStage Stage { get; set; }
- bool TryEvaluateDisplayName(DictionaryContextData contextData, IExecutionContext context);
Pipelines.ActionStep Action { get; set; }
}
@@ -285,25 +284,67 @@ namespace GitHub.Runner.Worker
}
- public bool TryEvaluateDisplayName(DictionaryContextData contextData, IExecutionContext context)
+ ///
+ /// Attempts to update the DisplayName.
+ /// As the "Try..." name implies, this method should never throw an exception.
+ /// Returns true if the DisplayName is already present or it was successfully updated.
+ ///
+ public bool TryUpdateDisplayName(out bool updated)
+ {
+ updated = false;
+
+ // REVIEW: This try/catch can be removed if some future implementation of EvaluateDisplayName and UpdateTimelineRecordDisplayName
+ // can make reasonable guarantees that they won't throw an exception.
+ try
+ {
+ // This attempt is only worthwhile at the "Main" stage.
+ // When the job starts, there's an initial attempt to evaluate the DisplayName. (see JobExtension::InitializeJob)
+ // During the "Pre" stage, we expect that no contexts will have changed since the initial evaluation.
+ // "Main" stage is handled here.
+ // During the "Post" stage, it no longer matters.
+ if (this.Stage == ActionRunStage.Main && EvaluateDisplayName(this.ExecutionContext.ExpressionValues, this.ExecutionContext, out updated))
+ {
+ if (updated)
+ {
+ this.ExecutionContext.UpdateTimelineRecordDisplayName(this.DisplayName);
+ }
+ }
+ }
+ catch (Exception ex)
+ {
+ Trace.Warning("Caught exception while attempting to evaulate/update the step's DisplayName. Exception Details: {0}", ex);
+ }
+
+ // For consistency with other implementations of TryUpdateDisplayName we use !string.IsNullOrEmpty below,
+ // but note that (at the time of this writing) ActionRunner::DisplayName::get always returns a non-empty string due to its fallback logic.
+ // In other words, the net effect is that this particular implementation of TryUpdateDisplayName will always return true.
+ return !string.IsNullOrEmpty(this.DisplayName);
+ }
+
+
+ ///
+ /// Attempts to evaluate the DisplayName of this IActionRunner.
+ /// Returns true if the DisplayName is already present or it was successfully evaluated.
+ ///
+ public bool EvaluateDisplayName(DictionaryContextData contextData, IExecutionContext context, out bool updated)
{
ArgUtil.NotNull(context, nameof(context));
ArgUtil.NotNull(Action, nameof(Action));
- // If we have already expanded the display name, there is no need to expand it again
- // TODO: Remove the ShouldEvaluateDisplayName check and field post m158 deploy, we should do it by default once the server is updated
+ updated = false;
+ // If we have already expanded the display name, don't bother attempting [re-]expansion.
if (_didFullyEvaluateDisplayName || !string.IsNullOrEmpty(Action.DisplayName))
{
- return false;
+ return true;
}
- bool didFullyEvaluate;
- _displayName = GenerateDisplayName(Action, contextData, context, out didFullyEvaluate);
+ _displayName = GenerateDisplayName(Action, contextData, context, out bool didFullyEvaluate);
- // If we evaluated fully mask any secrets
+ // If we evaluated, fully mask any secrets
if (didFullyEvaluate)
{
_displayName = HostContext.SecretMasker.MaskSecrets(_displayName);
+ updated = true;
}
context.Debug($"Set step '{Action.Name}' display name to: '{_displayName}'");
_didFullyEvaluateDisplayName = didFullyEvaluate;
diff --git a/src/Runner.Worker/JobExtension.cs b/src/Runner.Worker/JobExtension.cs
index 770db6643..580c9999d 100644
--- a/src/Runner.Worker/JobExtension.cs
+++ b/src/Runner.Worker/JobExtension.cs
@@ -306,13 +306,13 @@ namespace GitHub.Runner.Worker
}
}
- actionRunner.TryEvaluateDisplayName(contextData, context);
+ actionRunner.EvaluateDisplayName(contextData, context, out _);
jobSteps.Add(actionRunner);
if (prepareResult.PreStepTracker.TryGetValue(step.Id, out var preStep))
{
Trace.Info($"Adding pre-{action.DisplayName}.");
- preStep.TryEvaluateDisplayName(contextData, context);
+ preStep.EvaluateDisplayName(contextData, context, out _);
preStep.DisplayName = $"Pre {preStep.DisplayName}";
preJobSteps.Add(preStep);
}
@@ -328,10 +328,10 @@ namespace GitHub.Runner.Worker
if (message.ContextData.TryGetValue("inputs", out var pipelineContextData))
{
var inputs = pipelineContextData.AssertDictionary("inputs");
- if (inputs.Any())
+ if (inputs.Any())
{
context.Output($"##[group] Inputs");
- foreach (var input in inputs)
+ foreach (var input in inputs)
{
context.Output($" {input.Key}: {input.Value}");
}
diff --git a/src/Runner.Worker/JobExtensionRunner.cs b/src/Runner.Worker/JobExtensionRunner.cs
index 62fc3287f..eeb0a8393 100644
--- a/src/Runner.Worker/JobExtensionRunner.cs
+++ b/src/Runner.Worker/JobExtensionRunner.cs
@@ -1,6 +1,7 @@
using System;
using System.Threading.Tasks;
using GitHub.DistributedTask.ObjectTemplating.Tokens;
+using GitHub.DistributedTask.Pipelines.ContextData;
namespace GitHub.Runner.Worker
{
@@ -32,5 +33,18 @@ namespace GitHub.Runner.Worker
{
await _runAsync(ExecutionContext, _data);
}
+
+ public bool TryUpdateDisplayName(out bool updated)
+ {
+ updated = false;
+ return !string.IsNullOrEmpty(this.DisplayName);
+ }
+
+ public bool EvaluateDisplayName(DictionaryContextData contextData, IExecutionContext context, out bool updated)
+ {
+ updated = false;
+ return !string.IsNullOrEmpty(this.DisplayName);
+ }
+
}
}
diff --git a/src/Runner.Worker/StepsRunner.cs b/src/Runner.Worker/StepsRunner.cs
index dc3e80a25..05d21d4bb 100644
--- a/src/Runner.Worker/StepsRunner.cs
+++ b/src/Runner.Worker/StepsRunner.cs
@@ -1,12 +1,9 @@
using System;
using System.Collections.Generic;
-using System.Linq;
-using System.Text;
using System.Threading;
using System.Threading.Tasks;
using GitHub.DistributedTask.Expressions2;
using GitHub.DistributedTask.ObjectTemplating.Tokens;
-using GitHub.DistributedTask.Pipelines;
using GitHub.DistributedTask.Pipelines.ContextData;
using GitHub.DistributedTask.Pipelines.ObjectTemplating;
using GitHub.DistributedTask.WebApi;
@@ -14,8 +11,6 @@ using GitHub.Runner.Common;
using GitHub.Runner.Common.Util;
using GitHub.Runner.Sdk;
using GitHub.Runner.Worker.Expressions;
-using ObjectTemplating = GitHub.DistributedTask.ObjectTemplating;
-using Pipelines = GitHub.DistributedTask.Pipelines;
namespace GitHub.Runner.Worker
{
@@ -26,6 +21,8 @@ namespace GitHub.Runner.Worker
string DisplayName { get; set; }
IExecutionContext ExecutionContext { get; set; }
TemplateToken Timeout { get; }
+ bool TryUpdateDisplayName(out bool updated);
+ bool EvaluateDisplayName(DictionaryContextData contextData, IExecutionContext context, out bool updated);
Task RunAsync();
}
@@ -195,6 +192,12 @@ namespace GitHub.Runner.Worker
}
else
{
+ // This is our last, best chance to expand the display name. (At this point, all the requirements for successful expansion should be met.)
+ // That being said, evaluating the display name should still be considered as a "best effort" exercise. (It's not critical or paramount.)
+ // For that reason, we call a safe "Try..." wrapper method to ensure that any potential problems we encounter in evaluating the display name
+ // don't interfere with our ultimate goal within this code block: evaluation of the condition.
+ step.TryUpdateDisplayName(out _);
+
try
{
var templateEvaluator = step.ExecutionContext.ToPipelineTemplateEvaluator(conditionTraceWriter);
@@ -256,14 +259,6 @@ namespace GitHub.Runner.Worker
private async Task RunStepAsync(IStep step, CancellationToken jobCancellationToken)
{
- // Check to see if we can expand the display name
- if (step is IActionRunner actionRunner &&
- actionRunner.Stage == ActionRunStage.Main &&
- actionRunner.TryEvaluateDisplayName(step.ExecutionContext.ExpressionValues, step.ExecutionContext))
- {
- step.ExecutionContext.UpdateTimelineRecordDisplayName(actionRunner.DisplayName);
- }
-
// Start the step
Trace.Info("Starting the step.");
step.ExecutionContext.Debug($"Starting: {step.DisplayName}");
diff --git a/src/Test/L0/Worker/ActionRunnerL0.cs b/src/Test/L0/Worker/ActionRunnerL0.cs
index 31d074365..586e9dad2 100644
--- a/src/Test/L0/Worker/ActionRunnerL0.cs
+++ b/src/Test/L0/Worker/ActionRunnerL0.cs
@@ -3,20 +3,14 @@ using GitHub.DistributedTask.ObjectTemplating.Tokens;
using GitHub.DistributedTask.Pipelines;
using GitHub.DistributedTask.Pipelines.ContextData;
using GitHub.DistributedTask.WebApi;
-using GitHub.Runner.Common.Util;
using GitHub.Runner.Worker;
-using GitHub.Runner.Worker.Container;
using GitHub.Runner.Worker.Handlers;
using Moq;
using Newtonsoft.Json.Linq;
using System;
using System.Collections.Generic;
-using System.IO;
-using System.IO.Compression;
-using System.Reflection;
using System.Runtime.CompilerServices;
using System.Threading;
-using System.Threading.Tasks;
using Xunit;
using Pipelines = GitHub.DistributedTask.Pipelines;
@@ -149,11 +143,12 @@ namespace GitHub.Runner.Common.Tests.Worker
_context.Add("matrix", matrixData);
// Act
- // Should not do anything if we don't have a displayNameToken to expand
- var didUpdateDisplayName = _actionRunner.TryEvaluateDisplayName(_context, _actionRunner.ExecutionContext);
+ // Should report success with no updated required if there's already a valid display name.
+ var validDisplayName = _actionRunner.EvaluateDisplayName(_context, _actionRunner.ExecutionContext, out bool updated);
// Assert
- Assert.False(didUpdateDisplayName);
+ Assert.True(validDisplayName);
+ Assert.False(updated);
Assert.Equal(actionDisplayName, _actionRunner.DisplayName);
}
@@ -183,13 +178,51 @@ namespace GitHub.Runner.Common.Tests.Worker
// Act
// Should expand the displaynameToken and set the display name to that
- var didUpdateDisplayName = _actionRunner.TryEvaluateDisplayName(_context, _actionRunner.ExecutionContext);
+ var validDisplayName = _actionRunner.EvaluateDisplayName(_context, _actionRunner.ExecutionContext, out bool updated);
// Assert
- Assert.True(didUpdateDisplayName);
+ Assert.True(validDisplayName);
+ Assert.True(updated);
Assert.Equal(expectedString, _actionRunner.DisplayName);
}
+ [Fact]
+ [Trait("Level", "L0")]
+ [Trait("Category", "Worker")]
+ public void IgnoreDisplayNameTokenWhenDisplayNameIsExplicitlySet()
+ {
+ var explicitDisplayName = "Explcitly Set Name";
+
+ // Arrange
+ Setup();
+ var actionId = Guid.NewGuid();
+ var action = new Pipelines.ActionStep()
+ {
+ Name = "action",
+ Id = actionId,
+ DisplayName = explicitDisplayName,
+ DisplayNameToken = new BasicExpressionToken(null, null, null, "matrix.node"),
+ };
+
+ _actionRunner.Action = action;
+
+ var matrixData = new DictionaryContextData
+ {
+ ["node"] = new StringContextData("8")
+ };
+ _context.Add("matrix", matrixData);
+
+ // Act
+ // Should ignore the displayNameToken since there's already an explicit value for DisplayName
+ var validDisplayName = _actionRunner.EvaluateDisplayName(_context, _actionRunner.ExecutionContext, out bool updated);
+
+ // Assert
+ Assert.True(validDisplayName);
+ Assert.False(updated);
+ Assert.Equal(explicitDisplayName, _actionRunner.DisplayName);
+ }
+
+
[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
@@ -218,10 +251,11 @@ namespace GitHub.Runner.Common.Tests.Worker
// Act
// Should expand the displaynameToken and set the display name to that
- var didUpdateDisplayName = _actionRunner.TryEvaluateDisplayName(_context, _actionRunner.ExecutionContext);
+ var validDisplayName = _actionRunner.EvaluateDisplayName(_context, _actionRunner.ExecutionContext, out bool updated);
// Assert
- Assert.True(didUpdateDisplayName);
+ Assert.True(validDisplayName);
+ Assert.True(updated);
Assert.Equal("Run 8", _actionRunner.DisplayName);
}
@@ -246,10 +280,11 @@ namespace GitHub.Runner.Common.Tests.Worker
// Act
// Should expand the displaynameToken and set the display name to that
- var didUpdateDisplayName = _actionRunner.TryEvaluateDisplayName(_context, _actionRunner.ExecutionContext);
+ var validDisplayName = _actionRunner.EvaluateDisplayName(_context, _actionRunner.ExecutionContext, out bool updated);
// Assert
- Assert.True(didUpdateDisplayName);
+ Assert.True(validDisplayName);
+ Assert.True(updated);
Assert.Equal("Run TestImageName:latest", _actionRunner.DisplayName);
}
@@ -272,10 +307,11 @@ namespace GitHub.Runner.Common.Tests.Worker
// Act
// Should not do anything if we don't have context on the display name
- var didUpdateDisplayName = _actionRunner.TryEvaluateDisplayName(_context, _actionRunner.ExecutionContext);
+ var validDisplayName = _actionRunner.EvaluateDisplayName(_context, _actionRunner.ExecutionContext, out bool updated);
// Assert
- Assert.False(didUpdateDisplayName);
+ Assert.False(validDisplayName);
+ Assert.False(updated);
// Should use the pretty display name until we can eval
Assert.Equal("${{ matrix.node }}", _actionRunner.DisplayName);
}