NCronJob-Dev / NCronJob

A Job Scheduler sitting on top of IHostedService in dotnet.
https://docs.ncronjob.dev/
MIT License
158 stars 11 forks source link

Ensure parameter handling is coherent #132

Closed nulltoken closed 2 weeks ago

nulltoken commented 2 weeks ago

Pull request description

Minor change in parameter handling to ensure that a job parameter value can be overridden through the InstantJobRegistry.

PR meta checklist

Code PR specific checklist

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Files with missing lines Coverage Δ
src/NCronJob/Registry/JobRun.cs 83.82% <100.00%> (ø)
linkdotnet commented 2 weeks ago

Interesting - which case does that cover which wasn't covered before?

nulltoken commented 2 weeks ago

Interesting - which case does that cover which wasn't covered before?

This one. I stumbled onto this while digging deeper into what were the elements of a JobDefinition uniqueness.

Definition of a job with a standard "configuration" (passed through the parameter). Temporary overriding of that job "configuration" at runtime through the one-of execution of the job.

    [Fact]
    public async Task InstantJobCanOverrideInitiallyDefinedParameter()
    {
        ServiceCollection.AddNCronJob(
            n => n.AddJob<ParameterJob>(o => o.WithCronExpression("* * 31 2 *").WithParameter("Hello from AddNCronJob")));

        var provider = CreateServiceProvider();
        await provider.GetRequiredService<IHostedService>().StartAsync(CancellationToken);

        provider.GetRequiredService<IInstantJobRegistry>().RunInstantJob<ParameterJob>("Hello from InstantJob");

        var content = await CommunicationChannel.Reader.ReadAsync(CancellationToken);
        content.ShouldBe("Hello from InstantJob");
    }

The other tests were added/modified to have a more comprehensive suite in that context.

linkdotnet commented 2 weeks ago

Thanks for the clarification!

nulltoken commented 2 weeks ago

@linkdotnet Changelog has been updated