PR Feedback

This commit is contained in:
Francesco Renzi
2026-03-17 08:55:27 +00:00
committed by GitHub
parent f9919b2ba4
commit e3c0a6e119
7 changed files with 53 additions and 71 deletions

View File

@@ -21,6 +21,7 @@ namespace GitHub.Runner.Worker.Dap
private IDapDebugSession _session;
private CancellationTokenRegistration? _cancellationRegistration;
private volatile bool _started;
private bool _isFirstStep = true;
public bool IsActive => _session?.IsActive == true;
@@ -78,6 +79,23 @@ namespace GitHub.Runner.Worker.Dap
});
}
public async Task OnJobCompletedAsync()
{
if (_session != null && _started)
{
try
{
_session.OnJobCompleted();
}
catch (Exception ex)
{
Trace.Warning($"DAP OnJobCompleted error: {ex.Message}");
}
}
await StopAsync();
}
public async Task StopAsync()
{
if (_cancellationRegistration.HasValue)
@@ -108,7 +126,7 @@ namespace GitHub.Runner.Worker.Dap
_session?.CancelSession();
}
public async Task OnStepStartingAsync(IStep step, IExecutionContext jobContext, bool isFirstStep, CancellationToken cancellationToken)
public async Task OnStepStartingAsync(IStep step, IExecutionContext jobContext, CancellationToken cancellationToken)
{
if (!IsActive)
{
@@ -117,7 +135,9 @@ namespace GitHub.Runner.Worker.Dap
try
{
await _session.OnStepStartingAsync(step, jobContext, isFirstStep, cancellationToken);
bool isFirst = _isFirstStep;
_isFirstStep = false;
await _session.OnStepStartingAsync(step, jobContext, isFirst, cancellationToken);
}
catch (Exception ex)
{
@@ -142,23 +162,6 @@ namespace GitHub.Runner.Worker.Dap
}
}
public void OnJobCompleted()
{
if (!IsActive)
{
return;
}
try
{
_session.OnJobCompleted();
}
catch (Exception ex)
{
Trace.Warning($"DAP OnJobCompleted error: {ex.Message}");
}
}
private int ResolvePort()
{
var portEnv = Environment.GetEnvironmentVariable(PortEnvironmentVariable);

View File

@@ -12,8 +12,8 @@ namespace GitHub.Runner.Worker.Dap
Task WaitUntilReadyAsync(CancellationToken cancellationToken);
Task StopAsync();
void CancelSession();
Task OnStepStartingAsync(IStep step, IExecutionContext jobContext, bool isFirstStep, CancellationToken cancellationToken);
Task OnStepStartingAsync(IStep step, IExecutionContext jobContext, CancellationToken cancellationToken);
void OnStepCompleted(IStep step);
void OnJobCompleted();
Task OnJobCompletedAsync();
}
}

View File

@@ -193,11 +193,8 @@ namespace GitHub.Runner.Worker
catch (Exception ex)
{
Trace.Error($"Failed to start DAP debugger: {ex.Message}");
if (dapDebugger != null)
{
try { await dapDebugger.StopAsync(); } catch { }
}
dapDebugger = null;
jobContext.Error("Failed to start debugger.");
return await CompleteJobAsync(server, jobContext, message, TaskResult.Failed);
}
}
@@ -254,24 +251,18 @@ namespace GitHub.Runner.Worker
catch (OperationCanceledException) when (jobRequestCancellationToken.IsCancellationRequested)
{
Trace.Info("Job was cancelled before debugger client connected.");
try { await dapDebugger.StopAsync(); } catch { }
dapDebugger = null;
jobContext.Error("Job was cancelled before debugger client connected.");
return await CompleteJobAsync(server, jobContext, message, TaskResult.Canceled);
}
catch (Exception ex)
{
Trace.Error($"DAP debugger failed to become ready: {ex.Message}");
try { await dapDebugger.StopAsync(); } catch { }
dapDebugger = null;
}
}
// If debugging was requested but the debugger is not available, fail the job
if (jobContext.Global.EnableDebugger && dapDebugger == null)
{
var errorMessage = "The debugger failed to start or no debugger client connected in time.";
jobContext.Error(errorMessage);
return await CompleteJobAsync(server, jobContext, message, TaskResult.Failed);
// If debugging was requested but the debugger is not available, fail the job
var errorMessage = "The debugger failed to start or no debugger client connected in time.";
jobContext.Error(errorMessage);
return await CompleteJobAsync(server, jobContext, message, TaskResult.Failed);
}
}
// Run all job steps
@@ -314,10 +305,7 @@ namespace GitHub.Runner.Worker
runnerShutdownRegistration = null;
}
if (dapDebugger != null)
{
await dapDebugger.StopAsync();
}
await dapDebugger?.OnJobCompletedAsync();
await ShutdownQueue(throwOnFailure: false);
}

View File

@@ -51,16 +51,7 @@ namespace GitHub.Runner.Worker
jobContext.JobContext.Status = (jobContext.Result ?? TaskResult.Succeeded).ToActionResult();
var scopeInputs = new Dictionary<string, PipelineContextData>(StringComparer.OrdinalIgnoreCase);
bool checkPostJobActions = false;
IDapDebugger dapDebugger = null;
try
{
dapDebugger = HostContext.GetService<IDapDebugger>();
}
catch
{
// Debugger not available — continue without debugging
}
bool isFirstStep = true;
var dapDebugger = HostContext.GetService<IDapDebugger>();
while (jobContext.JobSteps.Count > 0 || !checkPostJobActions)
{
if (jobContext.JobSteps.Count == 0 && !checkPostJobActions)
@@ -238,20 +229,13 @@ namespace GitHub.Runner.Worker
else
{
// Pause for DAP debugger before step execution
if (dapDebugger != null)
{
await dapDebugger.OnStepStartingAsync(step, jobContext, isFirstStep, jobContext.CancellationToken);
isFirstStep = false;
}
await dapDebugger?.OnStepStartingAsync(step, jobContext, jobContext.CancellationToken);
// Run the step
await RunStepAsync(step, jobContext.CancellationToken);
CompleteStep(step);
if (dapDebugger != null)
{
dapDebugger.OnStepCompleted(step);
}
dapDebugger?.OnStepCompleted(step);
}
}
finally
@@ -279,10 +263,6 @@ namespace GitHub.Runner.Worker
Trace.Info($"Current state: job state = '{jobContext.Result}'");
}
if (dapDebugger != null)
{
dapDebugger.OnJobCompleted();
}
}
private async Task RunStepAsync(IStep step, CancellationToken jobCancellationToken)

View File

@@ -273,7 +273,7 @@ namespace GitHub.Runner.Common.Tests.Worker
var mockStep = new Mock<IStep>();
var mockJobContext = new Mock<IExecutionContext>();
await _debugger.OnStepStartingAsync(mockStep.Object, mockJobContext.Object, true, CancellationToken.None);
await _debugger.OnStepStartingAsync(mockStep.Object, mockJobContext.Object, CancellationToken.None);
mockSession.Verify(x => x.OnStepStartingAsync(mockStep.Object, mockJobContext.Object, true, CancellationToken.None), Times.Once);
@@ -306,7 +306,7 @@ namespace GitHub.Runner.Common.Tests.Worker
var mockStep = new Mock<IStep>();
var mockJobContext = new Mock<IExecutionContext>();
await _debugger.OnStepStartingAsync(mockStep.Object, mockJobContext.Object, true, CancellationToken.None);
await _debugger.OnStepStartingAsync(mockStep.Object, mockJobContext.Object, CancellationToken.None);
mockSession.Verify(x => x.OnStepStartingAsync(It.IsAny<IStep>(), It.IsAny<IExecutionContext>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()), Times.Never);
@@ -367,11 +367,10 @@ namespace GitHub.Runner.Common.Tests.Worker
var cts = new CancellationTokenSource();
await _debugger.StartAsync(cts.Token);
_debugger.OnJobCompleted();
await _debugger.OnJobCompletedAsync();
mockSession.Verify(x => x.OnJobCompleted(), Times.Once);
await _debugger.StopAsync();
mockServer.Verify(x => x.StopAsync(), Times.Once);
}
}
@@ -403,7 +402,7 @@ namespace GitHub.Runner.Common.Tests.Worker
var mockJobContext = new Mock<IExecutionContext>();
// Should not throw
await _debugger.OnStepStartingAsync(mockStep.Object, mockJobContext.Object, true, CancellationToken.None);
await _debugger.OnStepStartingAsync(mockStep.Object, mockJobContext.Object, CancellationToken.None);
await _debugger.StopAsync();
}

View File

@@ -8,6 +8,7 @@ using GitHub.DistributedTask.ObjectTemplating.Tokens;
using GitHub.DistributedTask.Pipelines.ObjectTemplating;
using GitHub.DistributedTask.WebApi;
using GitHub.Runner.Worker;
using GitHub.Runner.Worker.Dap;
using Moq;
using Xunit;
using Pipelines = GitHub.DistributedTask.Pipelines;
@@ -547,6 +548,11 @@ namespace GitHub.Runner.Common.Tests.Worker
var _stepsRunner = new StepsRunner();
_stepsRunner.Initialize(hc);
var mockDapDebugger = new Mock<IDapDebugger>();
mockDapDebugger.Setup(x => x.IsActive).Returns(false);
hc.SetSingleton(mockDapDebugger.Object);
await _stepsRunner.RunAsync(_jobEc);
Assert.Equal("Create custom image", snapshotStep.DisplayName);

View File

@@ -12,6 +12,7 @@ using GitHub.DistributedTask.ObjectTemplating.Tokens;
using GitHub.DistributedTask.WebApi;
using GitHub.Runner.Common.Util;
using GitHub.Runner.Worker;
using GitHub.Runner.Worker.Dap;
namespace GitHub.Runner.Common.Tests.Worker
{
@@ -61,6 +62,11 @@ namespace GitHub.Runner.Common.Tests.Worker
_stepsRunner = new StepsRunner();
_stepsRunner.Initialize(hc);
var mockDapDebugger = new Mock<IDapDebugger>();
mockDapDebugger.Setup(x => x.IsActive).Returns(false);
hc.SetSingleton(mockDapDebugger.Object);
return hc;
}