-
Notifications
You must be signed in to change notification settings - Fork 255
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
cancel context for each scenario #514
Conversation
15cb618
to
8309320
Compare
Not sure how to end-to-end test this though. |
8309320
to
023b342
Compare
Codecov Report
@@ Coverage Diff @@
## main #514 +/- ##
==========================================
- Coverage 82.94% 82.13% -0.81%
==========================================
Files 28 27 -1
Lines 3371 3320 -51
==========================================
- Hits 2796 2727 -69
- Misses 461 484 +23
+ Partials 114 109 -5
... and 4 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
This looks great. Would you mind adding a simple example code as well as part of this PR? I would prefer if each use case (like your new one, however it looks cool) has an appropriate example to follow otherwise some less experienced developers might be a bit lost in how to use it.
9c3a47d
to
ddb0fc4
Compare
I think this is a good idea, although it potentially can break existing scenarios that depend (implicitly or explicitly) on background tasks after the scenario has ended already. |
cucumber#549) * Update CI for go1.20 (cucumber#552) * fix(deps): update module github.com/cucumber/gherkin/go/v26 to v26.1.0 * Tidy modules --------- Co-authored-by: Viacheslav Poturaev <[email protected]> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@draganm for example, you can update func TestScenarioContext_After_cancelled(t *testing.T) {
ctxDone := make(chan struct{})
suite := TestSuite{
ScenarioInitializer: func(scenarioContext *ScenarioContext) {
scenarioContext.When(`^foo$`, func() { return })
scenarioContext.After(func(ctx context.Context, sc *Scenario, err error) (context.Context, error) {
go func() {
<-ctx.Done()
close(ctxDone)
}()
return ctx, nil
})
},
Options: &Options{
Format: "pretty",
TestingT: t,
FeatureContents: []Feature{
{
Name: "Scenario Context Cancellation",
Contents: []byte(`
Feature: dummy
Scenario: Context should be cancelled by the end of scenario
When foo
`),
},
},
},
}
require.Equal(t, 0, suite.Run(), "non-zero status returned, failed to run feature tests")
select {
case <-ctxDone:
return
case <-time.After(5 * time.Second):
assert.Fail(t, "failed to wait for context cancellation")
}
} |
Actually, I think this is good to merge, and I'll add a test in a follow-up commit. |
Hi @draganm, Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾 In return for this generous offer we hope you will:
On behalf of the Cucumber core team, |
🤔 What's changed?
Each Scenario gets a cancellable context that is cancelled at the end of the scenario.
⚡️ What's your motivation?
Cancelled context can be used to shut down mock services that are run in the scope of one Scenario (see #513)
🏷️ What kind of change is this?
♻️ Anything particular you want feedback on?
📋 Checklist:
This text was originally generated from a template, then edited by hand. You can modify the template here.