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

Support Glimmer #129

Closed
4 tasks
marcoow opened this issue Jul 11, 2017 · 6 comments
Closed
4 tasks

Support Glimmer #129

marcoow opened this issue Jul 11, 2017 · 6 comments

Comments

@marcoow
Copy link
Member

marcoow commented Jul 11, 2017

ember-test-selectors might already work (more or less) well with Glimmer but I'm not sure anyone has checked yet does not currently work with Glimmer. There are some things we need to get done before we can officially support it:

  • fix stripping of data-test-* attributes from templates
  • setup testing with Glimmer
  • don't patch Ember.Component when in a Glimmer app
  • don't register Babel plugins when in a Glimmer app (as dataAttributeBindings doesn't exist in Glimmer anyway)
@Turbo87
Copy link
Collaborator

Turbo87 commented Jul 11, 2017

I think we should wait until Glimmer has a somewhat stable testing story before tackling this problem

@marcoow
Copy link
Member Author

marcoow commented Jul 11, 2017

Actually this should be more or less unrelated to how Glimmer's testing story looks in detail though.

@marcoow
Copy link
Member Author

marcoow commented Aug 17, 2017

I think the only way to setup proper testing with Glimmer (and Ember) would be to use ember-cli-addon-tests. I experimented with that a bit (see https://github.com/simplabs/ember-test-selectors/compare/ember-cli-addon-tests) and it should work (and it should also be possible to add a fixture app that uses the Glimmer blueprint so we could test against that). However, the massive downside is that we cannot actually tests the app as it is running in the browser but would have to make HTTP request and assert on the response body. That means we can only

  • assert on whatever is in app.js; this basically means we have to assert on the Glimmer wire format (see e.g. https://github.com/simplabs/ember-test-selectors/compare/ember-cli-addon-tests#diff-60623ee8d9c9d63756a3abe26cb1c5bdR23) which is pretty bad as it will break whenever that changes and also requires all data-test- attributes we use in tests to be unique which makes the tests brittle
  • we can add FastBoot as another dependency so that we can assert on the rendered HTML as opposed to the contents of app.js; this is obviously not great neither as we're adding another dependency which adds quite a bit of complexity; asserting on the rendered HTML though would make asserting as such much better (e.g. using something like https://www.npmjs.com/package/cheerio would even allow us to select attributes on specific elements etc.)

At the moment (and I couldn't see how that would change in the future) I don't see any other solutions than the above 2…

@Turbo87
Copy link
Collaborator

Turbo87 commented Aug 18, 2017

I think we should wait until Glimmer has a somewhat stable testing story before tackling this problem

☝️

if we need to make this addon so much more complicated to support glimmer then IMHO it might make more sense to create a glimmer-test-selectors project instead to reduce the overhead for Ember users.

@marcoow
Copy link
Member Author

marcoow commented Aug 18, 2017

I don't think this situation will improve when Glimmer's testing story improves. After all, this is not a problem with Glimmer testing but with testing 2 very different Ember CLI projects. We'd be in the same situation if we needed to test with Ember CLI 1.x and and 2.x probably.

Forking glimmer-test-selectors also doesn't really seem like a viable solution. I think one of the key things we need to make sure in order for the Glimmer/Ember mid to long term plan to succeed is to not split the ecosystem (and community) in 2.

@Turbo87
Copy link
Collaborator

Turbo87 commented Oct 19, 2018

closing due to inactivity

if this ever becomes viable we can still reopen the issue...

@Turbo87 Turbo87 closed this as completed Oct 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants