[21.09] fix JobWrapper: use correct fail method

[bernt-matthias]

Add a test case and fix the job wrapper.

Since recently planemo was changed to use outputs_to_working_directory this branch of the code was used which used the wrong fail method. The consequence was that exit code, etc was lost.

fixes #14206

supersedes #14210

How to test the changes?

(Select all options that apply)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.
• Instructions for manual testing are as follows:
  1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

• I agree to license these contributions under Galaxy's current license.
• I agree to allow the Galaxy committers to license these and all my past contributions to the core galaxy codebase under the MIT license. If this condition is an issue, uncheck and just let us know why with an e-mail to galaxy-committers@lists.galaxyproject.org.

[bernt-matthias]

I added a branch in the code to check for the existence of the output files in any case (i.e. also if outputs_to_working_directory is not enabled or the extended metadata strategy is not used). So jobs should fail in any case if an output is absent.

Lets see if this has side effects

[bernt-matthias]

OK. There are side effects: in a non outputs_to_working setting (like the framework and api tests) the output may also be purged by the user and the job should not fail by this.

So I reverted and removed the test tool from the framework test.

Question: Would something like 131576f with an additional check if the dataset has been purged (no idea how to access the dataset here) be of interest?

[bernt-matthias]

Could need some help here.

• The integration test fails if the tool is not in the test tool config
• if the tool is included in the tool config file then the framework test fails (see previous commits), because in a non outputs_to_working setting tools deleting outputs are tolerated (one would need to distinguish it from users deleting outputs)

I guess we should have framework and integration tests and something like 131576f.. Also to have the same behavior in both settings (outputs_to_working one outputs to galaxy's files dir).

[mvdbeek]

You could access the config settings in $__app__.config.outputs_to_working_directory and set the test outputs as needed, or you can remove the tool test itself and run the test programmatically via the API interactor.

[bernt-matthias]

Thanks @mvdbeek .. but wouldn't it be better to have identical results in both cases (standard / outputs_to_working_directory), i.e. jobs should fail if the output has been purged?

There seem to be so many cases, e.g. what happens if the user purges a dataset during the runtime of a job in a outputs_to_working_directory setting. Will the job finish method restore the file .. but Galaxy does not know about it?

[bernt-matthias]

Maybe we should just mere the bugfix, i.e. the use of the correct fail method. And work on the test and everything else in a separate PR?