update toil and resurrect tests #300
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #300 +/- ##
=============================================
- Coverage 70.12% 68.12% -2.01%
+ Complexity 1060 1022 -38
=============================================
Files 47 48 +1
Lines 6069 6073 +4
Branches 801 801
=============================================
- Hits 4256 4137 -119
- Misses 1470 1592 +122
- Partials 343 344 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Split out https://ucsc-cgl.atlassian.net/browse/SEAB-6823 since I noticed we're using old surefire and failsafe plugins, but an update may be disruptive |
|
|
||
| /** | ||
| * @author dyuen | ||
| */ | ||
| @Ignore | ||
| // @Category(ToilOnlyTest.class) | ||
| @Tag(ToilOnlyTest.NAME) |
There was a problem hiding this comment.
Something weird is going on here with junit and how this is being picked up by surefire/failsafe, may want to work on this together with updating them in general since we're out-of-date in this repo
https://ucsc-cgl.atlassian.net/browse/SEAB-6823
denis-yuen
requested review from
a team,
david4096,
hyunnaye,
svonworl and
coverbeck
and removed request for
a team
dockstore-client/src/main/java/io/github/collaboratory/cwl/cwlrunner/ToilWrapper.java
Show resolved
Hide resolved
|
| @@ -35,9 +37,9 @@ public void checkForCWLDependencies(MetadataApi metadataApi) { | |||
| final ImmutablePair<String, String> pair1 = io.cwl.avro.Utilities | |||
| .executeCommand(Joiner.on(" ").join(Arrays.asList(s1)), false, com.google.common.base.Optional.absent(), | |||
| com.google.common.base.Optional.absent()); | |||
| final String toilVersion = pair1.getValue().trim(); | |||
| final String toilVersion = pair1.getKey().trim(); | |||
There was a problem hiding this comment.
I had to go look up that Utilities.executeCommand() returns a pair with stdOut and stdErr. For code readability, suggest a comment and/or a record.
Also, Toil changed where the version is displayed to stdOut from stdErr?
There was a problem hiding this comment.
Yeah on the second point.
I'll add a ticket since executeCommand is overloaded and used in a bunch of places.
https://ucsc-cgl.atlassian.net/browse/SEAB-6828
Description
Were toil integration tests ever hooked up to circle ci?
Possibly no, I think they must have been broken since we split out dockstore-cli into its own repository from the web-service. This PR updates Toil from 3.15.0 in 2018(!) to 7.0.0(!)
Amazingly, the tests still mostly work, so re-activating them.
Basically, they seem to just test running cwl as an alternative to cwltool and that's how the tests are implemented sharing a lot of code.
Can potentially follow-up with toil wdl testing or just use the environment to test metrics integration work.
Review Instructions
See that there's a handful of extra tests in CircleCI insights corresponding to the
integration-tests-toil-integration-testsjobIssue
https://ucsc-cgl.atlassian.net/browse/DOCK-2567
dockstore/dockstore#5964
Security
None
Please make sure that you've checked the following before submitting your pull request. Thanks!
./mvnw clean install