Refactor QuartzJobTransientRetryTests to use manual job instances consistently

[Copilot]

The component tests were creating manual job instances with test doubles but also scheduling separate instances with Quartz, creating ambiguity about which instance was being tested.

Changes

• Split test setup into two explicit methods:

  • CreateTestJobSetupWithoutScheduling: Tests Execute() logic directly without scheduler interaction
  • CreateTestJobSetupWithScheduling: Schedules job when verifying scheduler operations (e.g., job deletion)

• Added documentation clarifying when scheduling is necessary vs. when direct Execute() calls suffice

Before:

private async Task CreateTestJobSetup(...) {
    var job = CreateRunWorkflowJob(failingStarter);
    await scheduler.ScheduleJob(jobDetail, trigger);  // Always scheduled, even when unused
    return (job, failingStarter, context);
}

After:

// Transient exception tests - no scheduling needed
var (job, _, context) = await CreateTestJobSetupWithoutScheduling(...);

// Non-transient test - needs scheduling to verify deletion
var (job, _, context) = await CreateTestJobSetupWithScheduling(...);

All tests remain functionally equivalent but intent is now explicit.

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[Copilot]

Pull request overview

This PR refactors the test setup in QuartzJobTransientRetryTests to make the testing approach more explicit by distinguishing between tests that require Quartz scheduling and those that can test job execution logic directly.

Key Changes:

• Split CreateTestJobSetup into two distinct methods: CreateTestJobSetupWithoutScheduling for direct execution testing and CreateTestJobSetupWithScheduling for scheduler interaction testing
• Added comprehensive documentation explaining when each setup method should be used
• Updated test comments to clarify the reason for using each setup approach

Comments suppressed due to low confidence (1)

test/modules/scheduling/Elsa.Scheduling.Quartz.ComponentTests/QuartzJobTransientRetryTests.cs:121

• The two methods CreateTestJobSetupWithoutScheduling and CreateTestJobSetupWithScheduling contain significant code duplication. Consider extracting the common setup logic (lines 78-88 in the first method and 103-113 in the second) into a shared private method, then have each method call that shared method and only handle the scheduling-specific logic. This would reduce maintenance burden and make the code more DRY.

    private async Task<(RunWorkflowJob job, FailingWorkflowStarter failingStarter, IJobExecutionContext context)>
        CreateTestJobSetupWithoutScheduling(int failuresBeforeSuccess, Exception exception, string jobIdentifier)
    {
        var scheduler = await WorkflowServer.GetSchedulerAsync();
        var workflowStarter = Scope.ServiceProvider.GetRequiredService<IWorkflowStarter>();

        var failingStarter = new FailingWorkflowStarter(workflowStarter)
        {
            FailuresBeforeSuccess = failuresBeforeSuccess,
            ExceptionToThrow = exception
        };

        var job = CreateRunWorkflowJob(failingStarter);
        var (jobDetail, trigger) = CreateJobDetailAndTrigger(jobIdentifier);
        
        // Don't schedule with Quartz - we're testing the Execute method directly
        var context = CreateTestContext(scheduler, jobDetail, trigger);

        return (job, failingStarter, context);
    }

    /// <summary>
    /// Creates a test setup and schedules the job with Quartz.
    /// Use when testing behavior that involves scheduler operations (e.g., job deletion).
    /// </summary>
    private async Task<(RunWorkflowJob job, FailingWorkflowStarter failingStarter, IJobExecutionContext context)>
        CreateTestJobSetupWithScheduling(int failuresBeforeSuccess, Exception exception, string jobIdentifier)
    {
        var scheduler = await WorkflowServer.GetSchedulerAsync();
        var workflowStarter = Scope.ServiceProvider.GetRequiredService<IWorkflowStarter>();

        var failingStarter = new FailingWorkflowStarter(workflowStarter)
        {
            FailuresBeforeSuccess = failuresBeforeSuccess,
            ExceptionToThrow = exception
        };

        var job = CreateRunWorkflowJob(failingStarter);
        var (jobDetail, trigger) = CreateJobDetailAndTrigger(jobIdentifier);

        // Schedule the job with Quartz so the job can interact with the scheduler
        // (e.g., delete itself on non-transient exceptions)
        await scheduler.ScheduleJob(jobDetail, trigger);
        var context = CreateTestContext(scheduler, jobDetail, trigger);

        return (job, failingStarter, context);
    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.