Fix algorithm for generating output names when output names clash

[pvanheus]

When a scatter step in a workflow is chosen as a step in an output, the output names will naturally clash. Here is an example workflow:

#!/usr/bin/env cwl-runner
cwlVersion: v1.1
class: Workflow

inputs:
  range:
    type: int
outputs:
  output:
    type: File[]
    outputSource:
      generate_files/output

steps:
  generate_list:
    requirements:
      - class: InlineJavascriptRequirement
    run:
      class: ExpressionTool
      inputs:
        max: 
          type: int
          default: 100
      outputs:
        numbers:
          type: int[]
      expression: |
        ${
          var numberList = Array.apply(null, Array(inputs.max)).map(function(_, i) { return i});
          return { "numbers": numberList } 
         }
    in:
      max: range
    out:
      - numbers
  generate_files:
    requirements:
      - class: ScatterFeatureRequirement
    scatter: number
    run:
      class: CommandLineTool
      inputs:
        number:
          type: int
          inputBinding:
            position: 10
      baseCommand: [ echo ]
      stdout: output.txt
      outputs:
        output:
          type: stdout
    in:
      number: generate_list/numbers
    out:
      - output

The current cwltool logic for this case is here and it results in names in a list like output.txt, output.txt_2, output.txt_2_3 as the logic appends a number to the proposed target name instead of incrementing the number. Eventually cwltool crashes with a File name too long OSError exception. This patch changes the logic to always target the original target name (entry.target) and increment the number appended to the end so that names become output.txt, output.txt_2, output.txt_3 etc. It also adds a little comment explaining the logic.

[pvanheus]

Some help would be appreciated interpreting the test failure - I don't see how it is related to the change that this PR introduces.

[tetron]

@pvanheus that error is pretty obscure. I have restarted the build in case it was a transient failure: https://travis-ci.org/github/common-workflow-language/cwltool/jobs/725533973

[pvanheus]

Thanks @tetron - hit the same error again. I really don't get what this error means.

[tetron]

I am going to try running it locally and see what happens but I haven't gotten to it yet.

[pvanheus]

@tetron I tried to run the tests locally as per the README but py.test complained about the ----pyarg flag. Is there a way to perhaps do the testing in a Docker container?

[mr-c]

Thank you @pvanheus for this! I think I fixed the build on our side.

Can you add your example as a test case? range: 100 seems to run fast enough while still triggering the error without this fix

[codecov]

Codecov Report

Merging #1342 into main will decrease coverage by 2.22%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1342      +/-   ##
==========================================
- Coverage   77.71%   75.48%   -2.23%     
==========================================
  Files          41       41              
  Lines        7659     7658       -1     
  Branches     1949     1949              
==========================================
- Hits         5952     5781     -171     
- Misses       1215     1346     +131     
- Partials      492      531      +39     

Impacted Files	Coverage Δ
cwltool/process.py	86.48% <100.00%> (-0.82%)	⬇️
cwltool/workflow_job.py	74.72% <0.00%> (-11.09%)	⬇️
cwltool/checker.py	80.85% <0.00%> (-10.64%)	⬇️
cwltool/builder.py	80.87% <0.00%> (-8.47%)	⬇️
cwltool/command_line_tool.py	70.53% <0.00%> (-6.12%)	⬇️
cwltool/workflow.py	88.28% <0.00%> (-2.52%)	⬇️
cwltool/load_tool.py	82.02% <0.00%> (-1.39%)	⬇️
cwltool/argparser.py	86.23% <0.00%> (-1.38%)	⬇️
cwltool/update.py	72.54% <0.00%> (-1.31%)	⬇️
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 76c81f6...9733c5d. Read the comment docs.