Skip to content

[Testing] JobScopeTestExecutionListener overrides own JobExecution with id 123 #3881

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.

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

Open
kzander91 opened this issue Apr 9, 2021 · 2 comments
Labels
has: minimal-example Bug reports that provide a minimal complete reproducible example in: test status: feedback-provided Issues for which the feedback requested from the reporter was provided type: bug

Comments

@kzander91
Copy link

kzander91 commented Apr 9, 2021

Bug description
JobScopeTestExecutionListener registers a JobExecution with id 123 (created via MetaDataInstanceFactory#createJobExecution()) for every test using JobSynchronizationManager.register().
If the Job under test relies on Job-scoped parameters obtained from the execution stored in the JobSynchronizationManager, the 123rd job execution will fail, because when the Job starts and attempts to register its own JobExecution with id 123, the JobSynchronizationManager will not store it (since an execution with that id already exists).
In my specific case, this leads to expressions like #{jobParameters['myParam']} evaluating to null, because JobScope.getContext() will return the execution registered by the JobScopeTestExecutionListener.

I spent quite some time debugging this issue, because this only affects code that accesses the JobExecution via the synchronization manager and only after exactly 123 times.

Environment
Spring Batch 4.3.2, Spring Boot 2.4.4.

Steps to reproduce

  • Create Job with a @JobScoped Step that queries the job parameters.
  • Run the Job 123 (or more) times.

Expected behavior
The default id of a JobExecution (defined in MetaDataInstanceFactory#DEFAULT_JOB_EXECUTION_ID) should be a value that is not reached naturally by repeatedly calling a job. The same probably applies for instance ids and step execution ids.

Minimal Complete Reproducible example
I attached a demo project
demo.zip
Unzip it and run ./mvnw test.
The project contains a job scoped tasklet step that throws an exception if the job parameter is null.
A test runs this job, supplying the parameter, and repeats itself 125 times. When running the test, the repetition with execution id 123 fails.

Workaround
Place

public JobExecution getJobExecution() {
    return null;
}

somewhere in the test class. This causes JobScopeTestExecutionListener to not register its own JobExecution.

@kzander91 kzander91 added status: waiting-for-triage Issues that we did not analyse yet type: bug labels Apr 9, 2021
@fmbenhassine fmbenhassine added has: minimal-example Bug reports that provide a minimal complete reproducible example in: test labels Apr 14, 2021
@fmbenhassine
Copy link
Contributor

I agree, that should have taken some time to debug and figure out.. Anyway, thank you for reporting it and for providing a minimal example.

While the issue is valid, I think it is due to the usage of a job-scoped step bean which is not correct (similar to #3900). Changing your sample by moving the scoping from the step down to the tasklet makes the test pass:

@Bean
Job job() {
    return jobs.get("job") //
            .start(step()) //
            .build();
}

@Bean
Step step() {
    return steps.get("step") //
            .tasklet(getTasklet(null)) //
            .build();
}

@Bean
@StepScope
public Tasklet getTasklet(@Value("#{jobParameters['myParam']}") String myParam) {
    return (stepContribution, chunkContext) -> {
        if (!StringUtils.hasText(myParam)) {
            throw new IllegalStateException("myParam should never be null");
        }
        return RepeatStatus.FINISHED;
    };
}

Do you agree?

The default id of a JobExecution (defined in MetaDataInstanceFactory#DEFAULT_JOB_EXECUTION_ID) should be a value that is not reached naturally by repeatedly calling a job. The same probably applies for instance ids and step execution ids.

What do you suggest? We need a default value for those IDs, and any value could be reached by a repeatable test. So the issue would still happen for any default. That said, and pragmatically speaking, I don't see any alternative other than setting larger values for those default IDs.

@fmbenhassine fmbenhassine added status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter and removed status: waiting-for-triage Issues that we did not analyse yet labels Jun 15, 2023
@kzander91
Copy link
Author

kzander91 commented Jun 15, 2023

My project (and test suite) has evolved a lot in the past two years, I'm neither using @JobScoped steps anymore, nor am I running into this issue anymore, so can't verify that any changes would fix this (apart from the simplified demo project).

Regarding the default IDs in MetaDataInstanceFactory: Why not something like Long.MAX_VALUE or negative values?

@fmbenhassine fmbenhassine added status: feedback-provided Issues for which the feedback requested from the reporter was provided and removed status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter labels Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has: minimal-example Bug reports that provide a minimal complete reproducible example in: test status: feedback-provided Issues for which the feedback requested from the reporter was provided type: bug
Projects
None yet
Development

No branches or pull requests

2 participants