Add a CI job for ember-release with jQuery #5975
Conversation
dcyriller
changed the title
|
I am 👍 for adding a scenario to test with jquery-removed, but after updating the LTS targets etc. for releases this now needs rebased (and potentially a slight touchup) |
|
OK, I've pushed a few changes here:
Now that jQuery is being properly removed, this has a few test failures (19). Most seem directly related to testing jQuery functionality and can be fixed by adding a test helper to guard those tests when jQuery is missing, but others will need to be looked into to see whats going on. |
|
I've reworked unit and integration tests for the rest adapter. Previously, there were two different files: |
| server.post('/posts', function() { | ||
| return [201, { 'Content-Type': 'application/json' }, '']; | ||
| }); | ||
| if (hasjQuery) { |
There was a problem hiding this comment.
I've shamefully opted out of this last rest adapter integration test for fetch scenario. I've not been able to make it pass. The network call is not catched by pretender. I suspect an issue in Pretender.
There was a problem hiding this comment.
Seems fine to me, but would you mind making an issue to dig into it?
There was a problem hiding this comment.
Alright 👍
Opened here: #6083
| @@ -78,6 +78,9 @@ jobs: | |||
| - name: 'Ember Release' | |||
| if: NOT tag IS present AND NOT (branch ~= /^(emberjs:release|emberjs:lts|release|lts).*/) | |||
| env: EMBER_TRY_SCENARIO=ember-release | |||
| - name: 'Ember Release with jQuery' | |||
There was a problem hiding this comment.
Should we run with/without against an LTS maybe?
There was a problem hiding this comment.
Let's punt on that since we have to figure out running those things differently on azure anyway, which we seem to be gravitating towards a chron job for
|
Needs a quick rebase but otherwise 👍 LGTM and would like to ship :) |
|
I've had more trouble than expected with the rebase. But it should be good now 👍 |
With the recent switch from Query.ajax to fetch in the adapters code and the jQuery removal in Ember, this job aims at adding a small safety net.
* Add azure pipelines job * Ensure @ember/optional-features is present (otherwise setting `EMBER_OPTIONAL_FEATURES` has no effect) * Run tests without jQuery by default
From now on, dedicated CI jobs will run with and without jQuery. This lets us simplify the adapters tests a bit. Indeed, the same tests will run for fetch (without jQuery) and ajax (with jQuery).
...to run with both fetch and ajax. Previously, jQuery was always loaded in the test suite. So this file was not run with fetch. Now, it is run by default with fetch (not ajax). In CI, both scenarios should be run.
* Add a CI job for ember-release with jQuery With the recent switch from Query.ajax to fetch in the adapters code and the jQuery removal in Ember, this job aims at adding a small safety net. * Add azure pipelines job * Ensure @ember/optional-features is present (otherwise setting `EMBER_OPTIONAL_FEATURES` has no effect) * Run tests without jQuery by default * tests: Merge fetch and ajax adapters tests From now on, dedicated CI jobs will run with and without jQuery. This lets us simplify the adapters tests a bit. Indeed, the same tests will run for fetch (without jQuery) and ajax (with jQuery). * Refactor rest-adapter integration test... ...to run with both fetch and ajax. Previously, jQuery was always loaded in the test suite. So this file was not run with fetch. Now, it is run by default with fetch (not ajax). In CI, both scenarios should be run.
With the recent switch from
jQuery.ajaxtofetchin the adapters code and thejQueryremoval in Ember, this job aims at adding a small safety net.Note: in the PR diff, you may notice a ember-3.4 removal in
config/ember-tryfile: this config was unused in CI jobs. And I think it doesn't make much sense to activate it. Indeed,jQuerywas not yet removed inember-source@3.4.