Turn on more CWL tests

[adamnovak]

Changelog Entry

To be copied to the draft changelog by merger:

• CWL 1.2 tests 35, 39, 42, and 56 now run (Fix CWL 1.2 conformance test 35 to reliably succeed on Kubernetes #3538, Fix CWL 1.2 conformance test 39 to reliably succeed on Kubernetes #3539, Fix CWL 1.2 conformance test 42 to reliably succeed on Kubernetes #3540, Fix CWL 1.2 conformance test 56 to reliably succeed on Kubernetes #3541)
• Kubernetes batch system no longer runs the root job an extra time on the cluster if it is a CWL local job
• Job descriptions track the version number of the description, for debugging job description update logic
• Debug logging for promise tracking has been expanded
• CWL tools which use input files where the paths are specified in the tool definition will now have their input files imported properly

Closes #3538
Closes #3539
Closes #3540
Closes #3541

Reviewer Checklist

• Make sure it is coming from issues/XXXX-fix-the-thing in the Toil repo, or from an external repo.
  • If it is coming from an external repo, make sure to pull it in for CI with:

    contrib/admin/test-pr otheruser theirbranchname issues/XXXX-fix-the-thing
    

  • If there is no associated issue, create one.
• Read through the code changes. Make sure that it doesn't have:
  • Addition of trailing whitespace.
  • New variable or member names in camelCase that want to be in snake_case.
  • New functions without type hints.
  • New functions or classes without informative docstrings.
  • Changes to semantics not reflected in the relevant docstrings.
  • New or changed command line options for Toil workflows that are not reflected in docs/running/cliOptions.rst
  • New features without tests.
• Comment on the lines of code where problems exist with a review comment. You can shift-click the line numbers in the diff to select multiple lines.
• Finish the review with an overall description of your opinion.

Merger Checklist

• Make sure the PR passes tests.
• Make sure the PR has been reviewed since its last modification. If not, review it.
• Merge with the Github "Squash and merge" feature.
  • If there are multiple authors' commits, add Co-authored-by to give credit to all contributing authors.
• Copy its recommended changelog entry to the Draft Changelog.
• Append the issue number in parentheses to the changelog entry.

[adamnovak]

This is a lie; I've reproduced some of these failing with complaints about the promise-fiddling job at the end of the workflow running too soon.

[adamnovak]

This can reproduce the issue with test 39:

FAILED=0
for TRY_NUM in {1..5} ; do
    for ITER in {1..32} ; do
        OMP_NUM_THREADS=1 TOIL_AWS_ZONE=us-west-2a TOIL_AWS_SECRET_NAME=shared-s3-credentials TOIL_APPLIANCE_SELF=quay.io/ucsc_cgl/toil:5.4.0a1-1a69dc2607c73f3b1eb707a1bab22693071c07ad-py3.6 'cwltest' '--verbose' '--tool=toil-cwl-runner' '--test=/public/home/anovak/build/toil/src/toil/test/cwl/spec_v12/conformance_tests.yaml' '--timeout=2400' '--basedir=/public/home/anovak/build/toil/src/toil/test/cwl/spec_v12' '-n=39' '-j8' '--' '--disableCaching=False' '--clean=always' '--logDebug' --setEnv OMP_NUM_THREADS '--batchSystem=kubernetes' >iter${ITER}.log 2>&1 &
    done
    
    for ITER in {1..32} ; do
        wait
        echo "Status: $?"
    done
    
    for ITER in {1..32} ; do
        grep "All tests passed" "iter${ITER}.log"
        if [[ "${?}" == "1" ]] ; then
            echo "Failure observed in iter${ITER}.log!"
            FAILED=1
            break
        fi
    done
    
    if [[ "${FAILED}" == "1" ]] ; then
        break
    fi
    echo "No failures observed in try ${TRY_NUM}"
done

You may want to hack the --quiet option in cwltoil so it actually logs at debug level.

[adamnovak]

The failures are happening because the root CWLWorkflow job is managing to get its body run twice, which messes up all the cross-references to what promises ought to be filled in when, I think.

[adamnovak]

I think the problem is that we have the same job (CWLWorkflow) running at the same time on Kubernetes and locally, and they are racing and generally causing trouble.

[adamnovak]

OK, the real problem was that the Kubernetes batch system was mistaking local job 0 for a non-local job and also running it on Kubernetes, leading to races and maybe the whole-workflow-runs-twice issue?

[adamnovak]

I've added a fix for test 56 to this; we weren't quite recursing right when exploring the CWL workflow looking for tool definitions and File objects, so we were forgetting to import files that came along as part of tool definitions instead of workflow inputs. We also seemed to be laboring under the misapprehension that we could go get them when the tool ran, which is why things worked at all for single machine.

[adamnovak]

OK @DailyDreaming, I've addressed most of your comments and synced this up so it's the same commit as #3652.

I split out a couple of the utility functions, but I don't think it made much of a dent in the bigness of the file. We might want to pull out ToilPathMapper, ToilFsAccess, etc. into their own files.

I also still don't have unit tests for download_structure, and probably won't for the next 2 weeks. If we want to ship this sooner and we need them, someone else might need to write them. Otherwise I can write them when I get back.

[adamnovak]

OK, I've fixed the problem that failed the tests on the last merge with the main development line, and added a test for download_structure. This should pass and is ready for re-review.

[adamnovak]

Actually since Lon is out, I think I might just merge this when it passes, since I have addressed the comments.

This has been done.

[mr-c]

Can this be closed now that #3652 is merged?