-
Notifications
You must be signed in to change notification settings - Fork 59
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
Feature/controllers tests #126
Feature/controllers tests #126
Conversation
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.
Code review: ✅
These tests look really solid. Thank you for migrating these test cases. Much appreciated.
I have a couple of notes:
- For the deployment controller tests, the Before section has
By
parts that show up even before providing theContext
or theIt
part. This ends up with an odd ordering of the actions, which makes debugging a bit nasty. - Some descriptions seem to focus on the end result rather than what actually happens in that test case. We can discuss more about these in a call.
@edif2008 There is no restriction on how you should use them (for example it's ok to use The one general rule here is that |
No description provided.