Project

General

Profile

Actions

Bug #12934

closed

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

Added by Peter Amstutz almost 7 years ago. Updated almost 7 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
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 1 (0 open1 closed)

Task #12943: Review 12934-cwl-conformanceResolvedTom Clegg01/12/2018Actions

Related issues 1 (1 open0 closed)

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

Updated by Peter Amstutz almost 7 years ago

  • Status changed from New to In Progress
Actions #2

Updated by Peter Amstutz almost 7 years ago

  • Description updated (diff)
Actions #4

Updated by Peter Amstutz almost 7 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.

Actions #5

Updated by Peter Amstutz almost 7 years ago

  • Target version set to 2018-01-17 Sprint
Actions #6

Updated by Peter Amstutz almost 7 years ago

  • Assigned To set to Peter Amstutz
Actions #7

Updated by Peter Amstutz almost 7 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)

Actions #8

Updated by Tom Clegg almost 7 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

Actions #9

Updated by Peter Amstutz almost 7 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.

Actions #10

Updated by Peter Amstutz almost 7 years ago

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

Updated by Tom Clegg almost 7 years ago

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

Updated by Peter Amstutz almost 7 years ago

  • Status changed from In Progress to Resolved

Passing on ci.commonwl.org

Actions

Also available in: Atom PDF