Bug #12934

[CWL] Fix Arvados bugs revealed by newly added conformance tests

Added by Peter Amstutz almost 4 years ago. Updated over 3 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
01/12/2018
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-

Description

Three CWL conformance tests are failing:

  • Workflow defaults override tool defaults (update cwltool dependency)
  • Array results from file glob should be sorted (fix CollectionFsAccess)
  • "InitialWorkDirRequirement with a nested directory structure from another step" (haven't investigated)

Subtasks

Task #12943: Review 12934-cwl-conformanceResolvedTom Clegg


Related issues

Related to Arvados - Bug #12903: arvados-cwl-runner only loads cwl $import directives when run with `--local` New

Associated revisions

Revision 6b3cfe60
Added by Peter Amstutz over 3 years ago

Merge branch '12934-cwl-conformance' refs #12934

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <>

Revision c5795cc6
Added by Peter Amstutz over 3 years ago

Merge branch '12934-cwl-dir-output' refs #12934

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <>

Revision 06237272 (diff)
Added by Peter Amstutz over 3 years ago

Fix sdk/cwl unit test refs #12934

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <>

History

#1 Updated by Peter Amstutz almost 4 years ago

  • Status changed from New to In Progress

#2 Updated by Peter Amstutz almost 4 years ago

  • Description updated (diff)

#4 Updated by Peter Amstutz almost 4 years ago

The first two tests are trivial to fix, the 3rd test reveals the known problem that manifests can't represent empty directories. Current discussion is that crunch-run could scan for empty directories in the output and add a placeholder file so the directory is retained.

#5 Updated by Peter Amstutz almost 4 years ago

  • Target version set to 2018-01-17 Sprint

#6 Updated by Peter Amstutz almost 4 years ago

  • Assigned To set to Peter Amstutz

#7 Updated by Peter Amstutz almost 4 years ago

12934-cwl-conformance @ dc494faf90f5b3a83e73b8232c4150695dc25d46

However, one of the conformance tests themselves has a mistake:

https://github.com/common-workflow-language/common-workflow-language/pull/589

This converts the failing test into a "not supported" test because writable file/directory staging isn't supported (but we still ought to do it to get to 100% conformance, #12764)

#8 Updated by Tom Clegg almost 4 years ago

This looks like a debug printf to remove:

                       for _, bind := range binds {
                               mnt := runner.Container.Mounts[bind]
                               // Check if there is a bind for this
                               // directory, in which case assume we don't need .keep
                               runner.CrunchLog.Printf("%v %v", path, bind)
                               if (containerPath == bind || strings.HasPrefix(containerPath, bind+"/")) && mnt.PortableDataHash != "d41d8cd98f00b204e9800998ecf8427e+0" {
                                       return
                               }
                       }

The rest LGTM

#9 Updated by Peter Amstutz almost 4 years ago

Tom Clegg wrote:

This looks like a debug printf to remove:

[...]

The rest LGTM

Thanks. Unfortunately although the CWL conformance tests pass, the unit tests are broken so I still have some work to do.

#10 Updated by Peter Amstutz over 3 years ago

  • Target version changed from 2018-01-17 Sprint to 2018-01-31 Sprint

#11 Updated by Tom Clegg over 3 years ago

  • Related to Bug #12903: arvados-cwl-runner only loads cwl $import directives when run with `--local` added

#12 Updated by Peter Amstutz over 3 years ago

  • Status changed from In Progress to Resolved

Passing on ci.commonwl.org

Also available in: Atom PDF