-
-
Notifications
You must be signed in to change notification settings - Fork 20
Moving production code example out from test code #178
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
✅ Deploy Preview for cucumber-website-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Heya! That's an intentional part of the tutorial. The tutorial follows the red-green-refactor pattern. Actually extracting the code out is mentioned at the end and left as an exercise for the reader. https://cucumber.io/docs/guides/10-minute-tutorial/?lang=java#refactoring |
|
My concern is more about what kind of design we’re modelling while we’re teaching”: Putting production logic into step definitions as the starting point, for newcomers looks like an endorsed pattern, instead of a deliberate code smell. |
|
Doing the absolute minimum is a common TDD pattern. The intermediate "mess" is a feature of the process, not a bug. Also keep in mind that the purpose of the 10 minute tutorial is to get people started quickly. Not teach them about separation of concerns or refactoring just yet. From a practical perspective the tutorial also has to teach this (at present) in 4 different languages. So not fleshing out the refactor step makes it easier to maintain and keeps the tutorial short. So yes, it is not a perfect demonstration of BDD or TDD and it doesn't follow best practices. But that also wasn't the aim here. That said. I don't see any harm in fleshing out the refactor step and cleaning up the mess. Though this would have to be done in 4 languages. I realize it is a much bigger ask than what you've offered so far but would this be something you're willing to contribute? |
|
Im my PR, I already managed the 4 language variations. I did not notice the refactoring section in the end, what makes no sense after my changes, so with that change I have to update my PR. I think even a 10-minute tutorial should not promote intermediate mess. |
As an end state no. But as an intermediate step it is essential for teaching and learning.
If you think otherwise I'm happy to disagree on that and we can close the PR. But if you want to expand on the refactoring section that cleans up the mess, I would gladly accept that as a PR. |
|
Thanks for your clarifications and explanations. I do understand the red–green–refactor practice and thanks for explaining the original intentions of the "intermediate mess". My concern is that, in this specific 10-minute tutorial, the business logic is so trivial that designers fabricated an artificial “mess”, and present an anti-pattern in order to have something to refactor. Meanwhile the main purpose of this page is to demonstrate how Cucumber works in 10 minutes (e.g. how it generates step definitions), not to show a complete TDD/BDD course. With this in mind, I’d kindly ask you to reconsider my proposal. |
|
I'm afraid we'll have to disagree. |
🤔 What's changed?
isItFridaybusiness logic was presented in test code. AStepDefinitionfile should not contain business logic, but call it. In the10-minutes-tutorialI moved the logic out to a separated code file, (under a production related packagecom.domain.prod.example, where applicable) to emphasise its different nature.⚡️ What's your motivation?
There is no open issue for this yet.
My motivation was that it was misleading. Keeping business example in the Step Definition file disrupted the trilogy of Scenario Definition - Step Definition - Production codes. Following such tutorial can lead to an inelegant solution.
🏷️ What kind of change is this?
♻️ Anything particular you want feedback on?
I did not have Docusaurus server to run the
mdxfiles.📋 Checklist:
This text was originally generated from a template, then edited by hand. You can modify the template here.