-
Notifications
You must be signed in to change notification settings - Fork 738
Fix workflow onComplete and onError in the entry workflow #5366
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
Conversation
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
✅ Deploy Preview for nextflow-docs-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@jorgee can you help me move this PR forward when you have some time? I'm hoping to get it into a 24.10.x patch release
We need to fix these failing tests:
WorkflowMetadataTest > should populate workflow object FAILED
org.spockframework.runtime.SpockComparisonFailure at WorkflowMetadataTest.groovy:112
WorkflowMetadataTest > should access workflow script variables onComplete FAILED
org.spockframework.runtime.SpockComparisonFailure at WorkflowMetadataTest.groovy:174
WorkflowMetadataTest > should access workflow script variables onError FAILED
org.spockframework.runtime.SpockComparisonFailure at WorkflowMetadataTest.groovy:244
Currently these tests are trying to capture the stdout, and they are setting up the onComplete handler in a different way than it is actually used. I think they just need to be rewritten as e2e tests that check the console output for the printed lines, as other e2e tests do.
...fined in workflow script fix unit tests and workflow_oncomplete test Signed-off-by: jorgee <jorge.ejarque@seqera.io>
The changes were also breaking the integration_test workflow-oncomplete.nf. In the case, that the closure comes from the configuration, we need to change the closure delegate.
Two new functions added to manage when onComplete and onError are called from the configuration.
Two integration tests ('script-vars-oncomplete.nf' and 'script-vars-onerror.nf') have been added to check the case where callbacks are defined in the workflow.
1f834a0 to
6454605
Compare
5a93547 to
27345a6
Compare
f6a3696 to
49b58d2
Compare
b4b321e to
069653d
Compare
b7b4221 to
c1114bc
Compare
Close #5261
Tested with this script:
workflow { params.outdir = 'results' def local = 'local' workflow.onComplete { if( workflow.success ) log.info "Success! View results: $params.outdir" else log.info "Failure!" log.info "local variable: ${local}" } }The problem is that this change breaks the WorkflowMetadata unit tests. But these unit tests do not accurately represent how the onComplete/onError is defined
I could rewrite the unit tests to print to stdout and try to capture the stdout. But these tests probably just need to be e2e tests in order to test the actual script context