-
-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
error improvements #587
base: main
Are you sure you want to change the base?
error improvements #587
Conversation
@kinow Could you add a test? I'm not familiar with the testing framework |
b21c655
to
d4b2d3e
Compare
Sorry, saw this comment before approving. Let me take a look… |
found = true; | ||
} else if (found && (extractProcess(cwlFile) != CWLProcess.WORKFLOW)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, @mr-c. I tried writing a unit test for this, but I couldn't make the service to reach this line. The found
starts as false
. And unless it's set before, it will not enter this else if
block.
To make it eval to true
, I packed the CommandLineTool,
cwlVersion: v1.2
$graph:
- id: echo
class: CommandLineTool
baseCommand: echo
inputs: []
outputs: []
but that still results in false
since the code above already checks if it is a packed workflow, and only sets found
to true
in that case.
Did you reach this part of the code testing with the application running?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried with
cwlVersion: v1.2
# What type of CWL process we have in this document.
class: CommandLineTool
label: Process DL0 to DL3
doc: >
This tool (must become a workflow)
processes DL0 to DL3 simulated data.
baseCommand: echo
inputs:
metadata_queue:
type: File
inputBinding:
prefix: --metadata-queue
position: 1
outputs:
dl3:
type: File[]
outputBinding:
glob: '*.txt'
dl3_metadata:
type: stdout
irfs:
type: File[]
outputBinding:
glob: '*.txt'
irfs_metadata:
type: stdout
stdout: output.txt
And it still couldn't reach that part of the code. Here's the test:
diff --git a/src/test/java/org/commonwl/view/cwl/CWLServiceTest.java b/src/test/java/org/commonwl/view/cwl/CWLServiceTest.java
index 384fcd4..fe6993d 100644
--- a/src/test/java/org/commonwl/view/cwl/CWLServiceTest.java
+++ b/src/test/java/org/commonwl/view/cwl/CWLServiceTest.java
@@ -442,4 +442,24 @@ public class CWLServiceTest {
assertEquals(CWLProcess.COMMANDLINETOOL, steps.get("lobSTR").getRunType());
}
}
+
+ @Test
+ public void testExceptionRaisedForCommandLineTool() {
+ Exception thrown =
+ Assertions.assertThrows(
+ CWLNotAWorkflowException.class,
+ () -> {
+ CWLService cwlService =
+ new CWLService(
+ Mockito.mock(RDFService.class),
+ Mockito.mock(CWLTool.class),
+ Mockito.mock(GitConfig.class).licenseVocab(),
+ 999999);
+ cwlService.parseWorkflowNative(
+ Paths.get("src/test/resources/cwl/invalid_is_a_packed_command_line.cwl"), "");
+ });
+ assertEquals(
+ "File 'lobSTR-workflow.cwl' is over singleFileSizeLimit - 2 KB/0 bytes",
+ thrown.getMessage());
+ }
}
Just create the CWL file at src/test/resources/cwl/invalid_is_a_packed_command_line.cwl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(WIP test, I copied another test and was modifying it, slowly altering and re-running it)
d4b2d3e
to
5ac1fb1
Compare
(Clicked on Update branch -- forgot to check if there was an option to rebase...) |
Description
When the use provides a path to a CWL document that is not
class: Workflow
or a$graph
containing aclass: Workflow
, we need a better error message thanError: The workflow could not be found within the repository
Motivation and Context
Fixes #582
How Has This Been Tested?
I tested locally with the provided Git repository URL, branch, and path in the original issue.
Screenshots (if appropriate):
Types of changes
Checklist: