Skip to content

Allow accessibility tests to be skipped#815

Merged
monfresh merged 1 commit intomasterfrom
mb-tag-accessibility-specs
Dec 6, 2016
Merged

Allow accessibility tests to be skipped#815
monfresh merged 1 commit intomasterfrom
mb-tag-accessibility-specs

Conversation

@monfresh
Copy link
Contributor

@monfresh monfresh commented Dec 6, 2016

Why: Because they take 24 seconds to run, and we don't need to
check accessibility tests all the time when developing locally.

How: Add a fast_test command to the Makefile that skips tests
tagged with accessibility.

@monfresh monfresh self-assigned this Dec 6, 2016
@monfresh monfresh added this to the Sprint 20 milestone Dec 6, 2016
Copy link
Contributor

@jessieay jessieay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 comments from me, up to you whether to incorporate the feedback. Looks 💯 either way

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this tagging method. Might also make sense to include something in all of these accessibility specs that auto-includes this tag (and :js) so people don't need to remember to add this if/when future accessibility specs are added

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great idea, although I don't know how to do that without putting all accessibility tests in a single file. Do you know?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about changing the command line arguments to add --exclude-pattern "accessibility". That way as long as we keep the tests in the right directory we don't have to worry about tagging the tests?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea @zachmargolis -- I was just researching ways to programmatically add a flag to an example, but didn't find anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be helpful to tag the tests regardless, in case someone wants to only run the accessibility specs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it needs to be more specific: https://www.relishapp.com/rspec/rspec-core/v/3-5/docs/configuration/exclude-pattern

I'll play around with it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What did you try? Based on: https://www.relishapp.com/rspec/rspec-core/v/3-5/docs/configuration/exclude-pattern

I would expect this to work:

--exclude-pattern "**/features/accessibility/*_spec.rb"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I'm trying right now, @jessieay

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That worked.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take another look at the latest changes.

Makefile Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is neat -- should we document it somewhere in the README?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, will do.

@monfresh monfresh force-pushed the mb-tag-accessibility-specs branch from b71148c to 55ad28e Compare December 6, 2016 16:23
**Why**: Because they take 24 seconds to run, and we don't need to
check accessibility tests all the time when developing locally.

**How**: Add a `fast_test` command to the Makefile that skips tests
in the `accessibility` directory.
@monfresh monfresh force-pushed the mb-tag-accessibility-specs branch from 55ad28e to 1e513d0 Compare December 6, 2016 18:17
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So simple! LGTM! 👍

@monfresh monfresh merged commit 7e2e84d into master Dec 6, 2016
@monfresh monfresh deleted the mb-tag-accessibility-specs branch December 6, 2016 18:36
amoose pushed a commit that referenced this pull request Feb 24, 2017
**Why**: Because they take 24 seconds to run, and we don't need to
check accessibility tests all the time when developing locally.

**How**: Add a `fast_test` command to the Makefile that skips tests
in the `accessibility` directory.
amoose pushed a commit that referenced this pull request Feb 28, 2017
**Why**: Because they take 24 seconds to run, and we don't need to
check accessibility tests all the time when developing locally.

**How**: Add a `fast_test` command to the Makefile that skips tests
in the `accessibility` directory.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants