-
Notifications
You must be signed in to change notification settings - Fork 718
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
base: master
Are you sure you want to change the base?
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:
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 |
5a93547
to
27345a6
Compare
b4b321e
to
069653d
Compare
Close #5261
Tested with this script:
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