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

Deprecate testSelector() helper #134

Merged
merged 1 commit into from
Jul 14, 2017
Merged

Deprecate testSelector() helper #134

merged 1 commit into from
Jul 14, 2017

Conversation

marcoow
Copy link
Member

@marcoow marcoow commented Jul 14, 2017

This deprecates the testSelector helper and propagates using the respective attribute selectors directly instead. Besides obfuscating what's going on, the testSelector helper doesn't really do much.

@marcoow marcoow requested a review from Turbo87 July 14, 2017 12:19
find(testSelector('post-title')) // => find('[data-test-post-title]')
find(testSelector('resource-id', '2')) // => find('[data-test-resource-id="2"]')
find('[data-test-post-title]')
find('[data-test-resource-id="2"]')
Copy link
Member Author

@marcoow marcoow Jul 14, 2017

Choose a reason for hiding this comment

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

This diff is actually the perfect explanation of why I think the helper was a bad idea in the first place ;)

README.md Outdated
Once you've done that you can use the `testSelector()` function to create
a CSS/jQuery selector that looks up the right elements:
Once you've done that you can use an attribute selectors to look up the
elements:
Copy link
Collaborator

Choose a reason for hiding this comment

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

no an

addon/index.js Outdated
const { isNone, deprecate } = Ember;

const message = `Using the "testSelector" helper function is deprecated as it obfuscates the selectors, making the tests harder to follow.
Please write the actual abbribute selectors you need instead, e.g. "[data-test-my-element]" instead of "testSelector('my-element')".`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

abbribute sounds like a funny little thing 😜

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use instead of Please write sounds better IMHO

Copy link
Member Author

Choose a reason for hiding this comment

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

😞

@marcoow marcoow force-pushed the deprecate-test-helper branch from 17c3cb2 to a9713c8 Compare July 14, 2017 12:34
@marcoow
Copy link
Member Author

marcoow commented Jul 14, 2017

@Turbo87: fixed the surprisingly large amount of typos ;)

@Turbo87 Turbo87 changed the title deprecate testSelector helper Deprecate testSelector() helper Jul 14, 2017
@marcoow marcoow merged commit 65d0c38 into master Jul 14, 2017
@marcoow marcoow deleted the deprecate-test-helper branch July 14, 2017 12:52
@ryanto
Copy link

ryanto commented Aug 12, 2017

I just upgraded ember-test-selectors in one of our apps and this caught me by surprise. Wonder if we can make this helper opt in via config? I can PR if you like the idea...

cc @marcoow @Turbo87

@Turbo87
Copy link
Collaborator

Turbo87 commented Aug 13, 2017

@ryanto the function is/was essentially just a one liner, so it should be reasonable to just import it in your codebase as a test helper module

@lorcan
Copy link
Contributor

lorcan commented Aug 25, 2017

I wrote a codemod to transform code to the new style. Hope it's helpful :-)

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.

4 participants