Skip to content

🧹 Remove unnecessary EvaluateJobSnapshot #4099

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
lawrencegripper wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

🧹 Remove unnecessary EvaluateJobSnapshot #4099

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
92374ce
🧹 Remove unnecessary EvaluateJobSnapshot
lawrencegripper Nov 10, 2025
26f0fad
Fix up tests
lawrencegripper Nov 10, 2025
a552a36
Leave EvaluateJobSnapshotRequest method to satisfy interface
lawrencegripper Nov 10, 2025
1030a26
Retrigger ci
lawrencegripper Nov 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Runner.Worker/JobExtension.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ public async Task<List<IStep>> InitializeJob(IExecutionContext jobContext, Pipel
}

// Register custom image creation post-job step if the "snapshot" token is present in the message.
var snapshotRequest = templateEvaluator.EvaluateJobSnapshotRequest(message.Snapshot, jobContext.ExpressionValues, jobContext.ExpressionFunctions);
var snapshotRequest = message.Snapshot;
if (snapshotRequest != null)
{
var snapshotOperationProvider = HostContext.GetService<ISnapshotOperationProvider>();
Expand Down
4 changes: 2 additions & 2 deletions src/Sdk/DTPipelines/Pipelines/AgentJobRequestMessage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public AgentJobRequestMessage(
TemplateToken jobOutputs,
IList<TemplateToken> defaults,
ActionsEnvironmentReference actionsEnvironment,
TemplateToken snapshot,
Snapshot snapshot,
String messageType = JobRequestMessageTypes.PipelineAgentJobRequest)
{
this.MessageType = messageType;
Expand Down Expand Up @@ -240,7 +240,7 @@ public ActionsEnvironmentReference ActionsEnvironment
}

[DataMember(EmitDefaultValue = false)]
public TemplateToken Snapshot
public Snapshot Snapshot
Copy link
Collaborator

@ericsciple ericsciple Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we might need a new field to avoid a compatibility problem between the runner version and the server.

{
get;
set;
Expand Down
34 changes: 5 additions & 29 deletions src/Test/L0/Worker/JobExtensionL0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Threading;
using System.Threading.Tasks;
using GitHub.DistributedTask.ObjectTemplating.Tokens;
using GitHub.DistributedTask.Pipelines;
using GitHub.DistributedTask.Pipelines.ObjectTemplating;
using GitHub.DistributedTask.WebApi;
using GitHub.Runner.Worker;
Expand Down Expand Up @@ -486,46 +487,21 @@ public async Task EnsureNoSnapshotPostJobStep()
public Task EnsureSnapshotPostJobStepForStringToken()
{
var snapshot = new Pipelines.Snapshot("TestImageNameFromStringToken");
var imageNameValueStringToken = new StringToken(null, null, null, snapshot.ImageName);
var imageNameValueStringToken = new Snapshot(imageName: snapshot.ImageName);
return EnsureSnapshotPostJobStepForToken(imageNameValueStringToken, snapshot);
}

[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
public Task EnsureSnapshotPostJobStepForMappingToken()
{
var snapshot = new Pipelines.Snapshot("TestImageNameFromMappingToken");
var imageNameValueStringToken = new StringToken(null, null, null, snapshot.ImageName);
var mappingToken = new MappingToken(null, null, null)
{
{ new StringToken(null,null,null, PipelineTemplateConstants.ImageName), imageNameValueStringToken }
};

return EnsureSnapshotPostJobStepForToken(mappingToken, snapshot);
}

[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
public Task EnsureSnapshotPostJobStepForMappingToken_WithIf_Is_False()
{
var snapshot = new Pipelines.Snapshot("TestImageNameFromMappingToken", condition: $"{PipelineTemplateConstants.Success}() && 1==0", version: "2.*");
var imageNameValueStringToken = new StringToken(null, null, null, snapshot.ImageName);
var condition = new StringToken(null, null, null, snapshot.Condition);
var version = new StringToken(null, null, null, snapshot.Version);

var mappingToken = new MappingToken(null, null, null)
{
{ new StringToken(null,null,null, PipelineTemplateConstants.ImageName), imageNameValueStringToken },
{ new StringToken(null,null,null, PipelineTemplateConstants.If), condition },
{ new StringToken(null,null,null, PipelineTemplateConstants.CustomImageVersion), version }
};

return EnsureSnapshotPostJobStepForToken(mappingToken, snapshot, skipSnapshotStep: true);

return EnsureSnapshotPostJobStepForToken(snapshot, snapshot, skipSnapshotStep: true);
}

private async Task EnsureSnapshotPostJobStepForToken(TemplateToken snapshotToken, Pipelines.Snapshot expectedSnapshot, bool skipSnapshotStep = false)
private async Task EnsureSnapshotPostJobStepForToken(Snapshot snapshotToken, Pipelines.Snapshot expectedSnapshot, bool skipSnapshotStep = false)
{
using (TestHostContext hc = CreateTestContext())
{
Expand Down
Loading