Skip to content
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

Add test helpers #12

Merged
merged 1 commit into from
Mar 21, 2016
Merged

Add test helpers #12

merged 1 commit into from
Mar 21, 2016

Conversation

pdud
Copy link
Contributor

@pdud pdud commented Mar 18, 2016

Hey – thanks for the great addon!

I took a look at #8. The solution is very similar but not exactly as suggested in the last comment

  • The helper converts to [data-test-selector] not *[data-test-selector]
  • The helper usage is also testSelector('post-title') not testSelector('data-test-post-title') as in the last comment (but I think that was your original intention anyhow)

I also removed the StripTestSelectorsTransform from ember-cli-build so the tests would have the selectors.

I based the readme additions off ember-simple-auth test helper section.

I'd be happy to make any changes :)

name: 'strip-test-selectors',
plugin: StripTestSelectorsTransform
});

Copy link
Contributor

Choose a reason for hiding this comment

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

I also removed the StripTestSelectorsTransform from ember-cli-build so the tests would have the selectors.

Currently the tests assume that the [data-test-*] attributes are stripped. Removing the plugin here won't strip them anymore in the tests and that's why they are failing...

@pangratz
Copy link
Contributor

Thank you so much for tackling this @pdud!

I think removing the data-test-* from the argument passed to the helper was @marcoow's intention, so 👍 to that!

Since testSelector() only returns a string, I think it is ok to unit test it. Then the plugin needn't be removed from ember-cli-build.js and the tests should be green again.

@marcoow
Copy link
Member

marcoow commented Mar 18, 2016

Thanks @pdud, this is what I had in mind when opening #8. As @pangratz said, a unit test is probably the better choice for the testSelector helper.

Looking forward to getting this merged - will definitely help making tests cleaner and more concise!

The test helpers can be imported from the helpers/ember-test-selectors module in the application's namespace:

```javascript
import { testSelector } from '<app-name>/tests/helpers/ember-test-selectors';
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also promote the default export here...

@pdud
Copy link
Contributor Author

pdud commented Mar 18, 2016

Thank you very much for the suggestions. I've made those changes :)


```javascript
this.$(testSelector('post-title').click() // => this.$('[data-test-post-title]').click()
this.$(testSelector('selector', 'post-title').click() // => this.$('[data-test-selector="post-title"]').click()
Copy link
Contributor

Choose a reason for hiding this comment

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

There are closing )'s is missing here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😳 fixed now

- Adds test helpers for use in acceptance tests and integration tests
- Fixes #8
marcoow added a commit that referenced this pull request Mar 21, 2016
@marcoow marcoow merged commit ce4f701 into mainmatter:master Mar 21, 2016
@marcoow
Copy link
Member

marcoow commented Mar 21, 2016

🎉

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