Skip to content
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

Improve testability of SystemCommandTasklet #3955

Closed
scordio opened this issue Jun 30, 2021 · 2 comments
Closed

Improve testability of SystemCommandTasklet #3955

scordio opened this issue Jun 30, 2021 · 2 comments

Comments

@scordio
Copy link
Contributor

scordio commented Jun 30, 2021

Currently, the SystemCommandTasklet is not so test-friendly, mainly due to the usage of Runtime.getRuntime().exec(...):

Process process = Runtime.getRuntime().exec(command, environmentParams, workingDirectory);

For example, testing a job or a step with this tasklet defined does not work properly if the platform running the test does not have the target command available.

Would it make sense to decouple the Runtime dependency from this tasklet, so that a test can have more control over it? I would propose a pattern similar to the SystemExiter and JvmSystemExiter.

I am happy to submit a proposal in case the idea is accepted.

@scordio scordio added status: waiting-for-triage Issues that we did not analyse yet type: feature labels Jun 30, 2021
@fmbenhassine
Copy link
Contributor

Thank you for opening this feature request. This makes sense to me, something like CommandRunner and JvmCommandRunner could indeed decouple the command execution from the tasklet and make it possible to mock/stub the CommandRunner in tests. You are welcome to open a PR for that.

@scordio
Copy link
Contributor Author

scordio commented Jul 5, 2021

Thanks, @benas! I'll work on it.

@fmbenhassine fmbenhassine added this to the 5.0.0 milestone Sep 2, 2021
@fmbenhassine fmbenhassine added in: core and removed status: waiting-for-triage Issues that we did not analyse yet labels Sep 2, 2021
@fmbenhassine fmbenhassine modified the milestones: 5.0.0, 5.0.0-M6 Aug 31, 2022
@fmbenhassine fmbenhassine modified the milestones: 5.0.0-M6, 5.0.0-M7 Sep 20, 2022
@fmbenhassine fmbenhassine modified the milestones: 5.0.0-M7, 5.0.0-M8 Oct 4, 2022
fmbenhassine added a commit that referenced this issue Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants