Skip to content
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

Fix workflow onComplete and onError in the entry workflow #5366

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bentsherman
Copy link
Member

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

Copy link

netlify bot commented Oct 4, 2024

Deploy Preview for nextflow-docs-staging ready!

Name Link
🔨 Latest commit cfffeb3
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/673f514ab0ab0000083b3bd3
😎 Deploy Preview https://deploy-preview-5366--nextflow-docs-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@bentsherman
Copy link
Member Author

@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 <[email protected]>
@jorgee
Copy link
Contributor

jorgee commented Nov 21, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workflow onComplete/onError handlers don't work when defined in the entry workflow
3 participants