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

upgrade @ember/test-helpers #925

Merged
merged 2 commits into from
Aug 19, 2021
Merged

Conversation

stefanpenner
Copy link
Collaborator

@stefanpenner stefanpenner commented Aug 18, 2021

this also upgrades ember-cli, not because of this pull request but because ember-beta and ember-canary impose a minimum version of ember-cli, as they dropped the Ember global and older versions of ember-cli still reference it...

[fixes #926]

@stefanpenner
Copy link
Collaborator Author

node test failures I will investigate next (likely tomorrow AM)

FAIL packages/compat/tests/stage2.test.js (123.323s)
  ● stage2 build › engines with css › lazy engine css is imported

    assets/_engine_/lazy-engine.js
    Exected: "  if (macroCondition(!getGlobalConfig().fastboot?.isRunning)) {
    i(\"../../node_modules/lazy-engine/lazy-engine.css\");
      }"
    Received: "
    import { importSync as i, macroCondition, getGlobalConfig } from '@embroider/macros';
    let w = window;
    let d = w.define;·
      if (macroCondition(!getGlobalConfig().fastboot?.isRunning)) {
    i(\"../../node_modules/lazy-engine/lazy-engine/__COMPILED_STYLES__/lazy-engine.css\");
      }·
    d(\"lazy-engine/config/_environment_browser_\", function(){ return i(\"../../node_modules/lazy-engine/config/_environment_browser_.js\");});
    d(\"lazy-engine/config/environment\", function(){ return i(\"../../node_modules/lazy-engine/config/environment.js\");});
    d(\"lazy-engine/engine\", function(){ return i(\"../../node_modules/lazy-engine/engine.js\");});
    d(\"lazy-engine/package\", function(){ return i(\"../../node_modules/lazy-engine/package.json\");});······
    "

      718 |
      719 |     test('lazy engine css is imported', function () {
    > 720 |       expectFile('assets/_engine_/lazy-engine.js')
          |       ^
      721 |         .matches(`  if (macroCondition(!getGlobalConfig().fastboot?.isRunning)) {
      722 | i(\"../../node_modules/lazy-engine/lazy-engine.css\");
      723 |   }`);

      at Object.<anonymous> (packages/compat/tests/stage2.test.ts:720:7)

FAIL packages/compat/tests/addon-styles.test.js (22.146s)
  ● addon.styles tests › all addon CSS gets convert to implicit-styles

    node_modules/my-addon3/package.json
    Exected: "./my-addon3.css"
    Received: ["./my-addon3/__COMPILED_STYLES__/my-addon3.css", "./my-addon3/__COMPILED_STYLES__/nested/inner.css", "./my-addon3/__COMPILED_STYLES__/outer.css"]

       95 |
       96 |   test(`all addon CSS gets convert to implicit-styles`, function () {
    >  97 |     let implicitStyles = expectFile('node_modules/my-addon3/package.json').json().get('ember-addon.implicit-styles');
          |                          ^
       98 |     implicitStyles.includes('./my-addon3.css');
       99 |     implicitStyles.includes('./outer.css');
      100 |     implicitStyles.includes('./nested/inner.css');

      at Object.<anonymous> (packages/compat/tests/addon-styles.test.ts:97:26)


Test Suites: 2 failed, 40 passed, 42 total
Tests:       2 failed, 597 passed, 599 total
Snapshots:   0 total
Time:        490.237s

@stefanpenner
Copy link
Collaborator Author

Reading through the failures and the original changes suggests this is a safe change, and the tests merely need to be updated. Specifically it appears that the intermediate output has changed, but the consuming code is also correctly update to account for said change, resulting in the two errors we see now.

@rwjblue is on vacation, and he has most context. I'll sync with him when he returns, but I will likely update these tests and move on. I am pretty confident my analysis is sufficient.

@stefanpenner stefanpenner force-pushed the upgrade-ember-test-helpers branch from 6e8adae to 1a83cc0 Compare August 19, 2021 16:01
Unfortunately due to an issue, /test-packages/support/package.json can only
be upgraded to 3.17, as an issue manifests with 3.18. 

This issue still needs to be diagnosed.
@stefanpenner stefanpenner force-pushed the upgrade-ember-test-helpers branch from 4a23d4c to 5434a36 Compare August 19, 2021 20:40
@stefanpenner stefanpenner merged commit 153a074 into master Aug 19, 2021
@stefanpenner stefanpenner deleted the upgrade-ember-test-helpers branch August 19, 2021 21:28
mydea pushed a commit to mydea/embroider that referenced this pull request Aug 20, 2021
* upgrade @ember/test-helpers

* Upgrade ember-cli to 3.27 where possible

Unfortunately due to an issue, /test-packages/support/package.json can only
be upgraded to 3.17, as an issue manifests with 3.18. 

This issue still needs to be diagnosed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI is broken due to beta/canary dropping the Ember global
3 participants