Skip to content

Add guidelines for feature specs #37

Closed
wants to merge 4 commits into from

7 participants

@jferris
thoughtbot, inc. member
jferris commented Nov 30, 2012
  • We lost some conventions when we left cucumber
  • Attempts to reboot our conventions with capybara/rspec dsl
  • Put together during the Friday dev discussion
@gabebw gabebw commented on an outdated diff Nov 30, 2012
style/README.md
@@ -164,11 +167,19 @@ Testing
* Order factory attributes: implicit attributes, explicit attributes,
child factory definitions. Each section's attributes are alphabetical.
* Order factory definitions alphabetically by factory name.
+* Place helper methods for feature specs directly in a `Features` module
@gabebw
thoughtbot, inc. member
gabebw added a note Nov 30, 2012

maybe "top-level Features module"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jferris jferris Add guidelines for feature specs
* We lost some conventions when we left cucumber
* Attempts to reboot our conventions with capybara/rspec dsl
3da34d7
@croaky croaky commented on an outdated diff Dec 1, 2012
style/README.md
* Prefer `eq` to `==` in RSpec.
* Separate setup, exercise, verification, and teardown phases with newlines.
* Use an `it` example for each execution path through the method.
* Use one factories.rb file per project.
* Use [stubs and spies](http://goo.gl/EciDJ) (not mocks) in isolated tests.
+* Use a single level of abstraction within scenarios
+* Use names like `ROLE_ACTION_spec.rb`, such as
+ `user_changes_password_spec.rb`, for feature spec file names
+* Use only one `feature` block per feature spec file
+* Use scenario titles that describe the success and failure paths
+* Use spec/features to store feature specs
+* Use spec/support/features for support code related to feature specs
@croaky
thoughtbot, inc. member
croaky added a note Dec 1, 2012

Style for the guidelines: periods at end of sentences.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@gabebw gabebw commented on the diff Dec 3, 2012
style/README.md
@@ -164,11 +167,20 @@ Testing
* Order factory attributes: implicit attributes, explicit attributes,
child factory definitions. Each section's attributes are alphabetical.
* Order factory definitions alphabetically by factory name.
+* Place helper methods for feature specs directly in a top-level `Features`
@gabebw
thoughtbot, inc. member
gabebw added a note Dec 3, 2012

Additionally:

* Include the Features module in RSpec scenarios with `:type => feature` set
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@gylaz gylaz and 4 others commented on an outdated diff Dec 3, 2012
style/README.md
* Prefer `eq` to `==` in RSpec.
* Separate setup, exercise, verification, and teardown phases with newlines.
* Use an `it` example for each execution path through the method.
* Use one factories.rb file per project.
* Use [stubs and spies](http://goo.gl/EciDJ) (not mocks) in isolated tests.
+* Use a single level of abstraction within scenarios
+* Use names like `ROLE_ACTION_spec.rb`, such as
+ `user_changes_password_spec.rb`, for feature spec file names
@gylaz
thoughtbot, inc. member
gylaz added a note Dec 3, 2012

Is there a risk of redundancy here? In situations when most specs require user to be logged in, then all the files will be prepended with user_.

@joshuaclayton
thoughtbot, inc. member
joshuaclayton added a note Dec 3, 2012

Great point - Swoop is in that situation currently, so I omitted the role from the file name. Interested in other peoples' thoughts on this.

@gabebw
thoughtbot, inc. member
gabebw added a note Dec 3, 2012

I think there's value in knowing that everything requires a user.

Is it valuable to have a visitor_changes_password_spec.rb, or is that just a failure case in user_changes_password? At what point does it get extracted to another file?

@mike-burns
thoughtbot, inc. member
mike-burns added a note Dec 3, 2012

When it makes sense to break a guideline, you should break it.

@jferris
thoughtbot, inc. member
jferris added a note Dec 5, 2012

Agreed. I think the rules we have make sense for most applications which have at least visitor/user/admin, but if the guideline makes no sense for an application, don't follow it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mike-burns mike-burns and 1 other commented on an outdated diff Dec 3, 2012
style/README.md
* Prefer `eq` to `==` in RSpec.
* Separate setup, exercise, verification, and teardown phases with newlines.
* Use an `it` example for each execution path through the method.
* Use one factories.rb file per project.
* Use [stubs and spies](http://goo.gl/EciDJ) (not mocks) in isolated tests.
+* Use a single level of abstraction within scenarios
+* Use names like `ROLE_ACTION_spec.rb`, such as
+ `user_changes_password_spec.rb`, for feature spec file names
+* Use only one `feature` block per feature spec file
+* Use scenario titles that describe the success and failure paths
@mike-burns
thoughtbot, inc. member
mike-burns added a note Dec 3, 2012

Should we have a guideline on how much to test--specifically, should we test all success and failure paths from a feature test?

@jferris
thoughtbot, inc. member
jferris added a note Dec 5, 2012

I think that topic is too complicated to cover in a few bullet points.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jferris
thoughtbot, inc. member
jferris commented Dec 5, 2012

@croaky @gabebw periods and :type => feature guideline added. Ready for re-review.

@calebthompson
thoughtbot, inc. member

:feature

thoughtbot, inc. member

Fixed.

@croaky
thoughtbot, inc. member
croaky commented Dec 5, 2012

Ready to merge.

@croaky
thoughtbot, inc. member
croaky commented Jan 7, 2013

This was merged in 75307b0

@croaky croaky closed this Jan 7, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.