Skip test step execution if --dry-run is specified #1220
Conversation
adrian-baker
changed the title
There was a problem hiding this comment.
Hi! Thanks for the bug report. I don't think this is the proper solution though.
Test cases are created in Runner#createTestCaseForPickle. Rather then passing the dry-run flag between components it can be given to the TestCase right away through its constructor.
| @@ -84,6 +84,25 @@ public void hooks_execute_also_after_failure() throws Throwable { | |||
| } | |||
|
|
|||
| @Test | |||
| public void steps_are_not_executed_on_dry_run() throws Throwable { | |||
There was a problem hiding this comment.
This test verifies two things. I think it should make into two tests.
|
|
||
| public TestCase(List<TestStep> testSteps, PickleEvent pickleEvent) { |
There was a problem hiding this comment.
Because this is an api package, should the ctor with the current signature be kept for backward compatibility? @mpkorstanje WDYT?
There was a problem hiding this comment.
Along with adding a deprecation tag that would be the correct course of of action.
In the long run (and I'm only just starting to think about this) I'd like TestCase to be an interface so we can exclude the constructors and run method from the public api. These methods aren't needed as part of the event bus protocol.
There was a problem hiding this comment.
Can we push this as a extra commit on this feature branch so we don't have to bother Adrian with this?
There was a problem hiding this comment.
I can't. Permissions have apparently not been set.
There was a problem hiding this comment.
I've pushed my suggested changes to #1223. I reckon we can finish up this discussion the PR for that branch. I've got no further objections to this branch being merged.
There was a problem hiding this comment.
Great, thanks. Yes wasn't comfortable with having to change an api package, but in the current state didn't seem avoidable.
|
Hi @adrian-baker, 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, |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Details
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: