[WIP]: upgrade ember-cli to 2.4 #212
Conversation
ember 1.13 uses obj[key] to set a key, however, need to use Ember.set
| let attributes = assign({}, config.APP); | ||
| attributes = assign(attributes, attrs); // use defaults, but you can override; | ||
| let attributes = Ember.merge({}, config.APP); | ||
| attributes = Ember.merge(attributes, attrs); // use defaults, but you can override; |
There was a problem hiding this comment.
Why is this change necessary? I thought merge was deprecated for assign
There was a problem hiding this comment.
This just came from running ember init on 2.4
There was a problem hiding this comment.
ohh right sorry read this wrong!
|
Looks to be a bunch of failing tests on Travis. Are you seeing them locally? |
|
@hhff yeah seeing them locally as well. Some have to do with backburner and the likes. This PR was mainly to show trying to upgrade ember-cli to 2.4 and leaving ember where it is at. You mentioned support for 1.10. I wonder if to get the tests passing, we would need to take an approach like this. I haven't been around the ember community long enough to understand how to maintain backwards compat and get all of these tests passing. |
|
@snewcomer im cool with switching to the ember-power-select way and dropping 1.x! Let's do that! |
|
|
||
| assert.throws(() => { | ||
| try { |
There was a problem hiding this comment.
assert.throws failes after ember 2.11 so wrap try catch. However, this method does not work in the infinity-loader unit test.
There was a problem hiding this comment.
| @@ -88,7 +89,7 @@ test('it uses the provided scrollable element', function(assert) { | |||
| assert.equal(scrollable[0], $("#content")[0]); | |||
| }); | |||
|
|
|||
| test('it throws error when scrollable element is not found', function(assert) { | |||
| skip('it throws error when scrollable element is not found', function(assert) { | |||
| assert.expect(1); | |||
|
|
|||
| this.subject({scrollable: "#nonexistent"}); | |||
There was a problem hiding this comment.
wrapping try catch around this only passes after reload of test. Fails on first run though. So assuming state issue. Need to dig further.
There was a problem hiding this comment.
Have tried many solutions here except ember-qunit-assert-helpers sol'n. I need to upgrade the test suite to be able to use this.
There was a problem hiding this comment.
Technically this error throws in didInsertElement.
Could we try something more specific to that method?
try {
this.didInsertElement();
} catch
or something like that?
There was a problem hiding this comment.
Brilliant! Yep that worked. pushed
|
@hhff yayyy! Here is a passing build tests against 2.4 and above. Used firefox to run the tests on travis as well - seen other addons and apps drop phantom js from their test suite as well. Let me know what you think. Still have a few tests to circle back on (that I skipped) that use assert.throws. |
|
I'm guessing to get these skipped (infinity-loader unit test) tests to pass I need to upgrade other deps including ember-qunit and potentially changing the acceptance tests to use |
| import RouteMixin from 'ember-infinity/mixins/route'; | ||
| import { module, test } from 'qunit'; | ||
|
|
||
| module('RouteMixin'); | ||
|
|
||
| const assign = Ember.assign || Ember.merge; | ||
|
|
||
| let EO = Ember.Object.create.bind(Ember.Object); |
There was a problem hiding this comment.
Doing this failed on test runs.
Here is a WIP to upgrade ember-cli to 2.4
Failing test - https://github.com/hhff/ember-infinity/blob/master/tests/acceptance/infinity-route-with-embedded-route-test.js#L19