[DO NOT MERGE] flaky test investigation

.github/workflows/ci-components.yml

@@ -70,6 +70,7 @@ jobs:
       matrix:
         try-scenario:
           - ember-lts-4.12
+          # - hds-ember
           # - ember-release
           - ember-beta
           # - ember-canary

packages/components/src/components/hds/advanced-table/index.ts

@@ -10,6 +10,7 @@ import { tracked } from '@glimmer/tracking';
 import { guidFor } from '@ember/object/internals';
 import { service } from '@ember/service';
 import { modifier } from 'ember-modifier';
+import { isDestroyed, isDestroying } from '@ember/destroyable';
 import HdsAdvancedTableTableModel from './models/table.ts';
 
 import type Owner from '@ember/owner';
@@ -559,7 +560,7 @@ export default class HdsAdvancedTable extends Component<HdsAdvancedTableSignatur
 
     if (reorderedMessageText !== undefined) {
       this.reorderedMessageText = reorderedMessageText;
-    } else {
+    } else if (!isDestroyed(this) && !isDestroying(this)) {
       const newPosition = insertedAt + 1;
       const translatedReorderedMessageText = this.hdsIntl.t(
         'hds.advanced-table.reordered-message',

packages/components/src/components/hds/breadcrumb/item.ts

@@ -44,10 +44,8 @@ export default class HdsBreadcrumbItem extends Component<HdsBreadcrumbItemSignat
     }
   }
 
-  async resolveLinkToExternal() {
-    this.linkToExternal = await hdsResolveLinkToExternal(
-      this.args.isRouteExternal
-    );
+  resolveLinkToExternal() {
+    this.linkToExternal = hdsResolveLinkToExternal(this.args.isRouteExternal);
   }
 
   /**

packages/components/src/components/hds/interactive/index.ts

@@ -44,10 +44,8 @@ export default class HdsInteractive extends Component<HdsInteractiveSignature> {
     }
   }
 
-  async resolveLinkToExternal() {
-    this.linkToExternal = await hdsResolveLinkToExternal(
-      this.args.isRouteExternal
-    );
+  resolveLinkToExternal() {
+    this.linkToExternal = hdsResolveLinkToExternal(this.args.isRouteExternal);
   }
   /**
    * Determines if a @href value is "external" (it adds target="_blank" rel="noopener noreferrer")

packages/components/src/services/hds-intl.ts

@@ -4,7 +4,7 @@
  */
 
 import Service from '@ember/service';
-import { getOwner } from '@ember/owner';
+import { service } from '@ember/service';
 import { isPresent } from '@ember/utils';
 import { assert } from '@ember/debug';
 
@@ -18,18 +18,7 @@ export type HdsIntlTOptions = FormatMessageParameters[1] & {
 };
 
 export default class HdsIntlService extends Service {
-  get intl(): IntlService | undefined {
-    const owner = getOwner(this);
-
-    if (
-      typeof owner?.factoryFor === 'function' &&
-      owner.factoryFor('service:intl')
-    ) {
-      return owner.lookup('service:intl');
-    }
-
-    return undefined;
-  }
+  @service('intl') declare intl: IntlService;
 
   t(key: string, options: HdsIntlTOptions): string {
     const { default: defaultString, ...restOptions } = options;

packages/components/src/utils/hds-resolve-link-to-external.ts

@@ -5,26 +5,29 @@
 
 import { LinkTo } from '@ember/routing';
 import { assert } from '@ember/debug';
+import {
+  dependencySatisfies,
+  macroCondition,
+  importSync,
+} from '@embroider/macros';
 
 /**
  * Resolves the correct component to use for the `LinkTo`.
  *
  * @param isRouteExternal - If true, will return the `LinkToExternal` component. If `ember-engines` is not installed, an assertion will be thrown.
- * @returns A promise resolving to the correct component to use for the `LinkTo`.
+ * @returns The correct component to use for the `LinkTo`.
  */
-export async function hdsResolveLinkToExternal(
+export function hdsResolveLinkToExternal(
   isRouteExternal?: boolean
-): Promise<typeof LinkTo> {
+): typeof LinkTo {
   if (isRouteExternal) {
-    try {
-      // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
-      const mod = await import(
-        // @ts-expect-error: we list this as optional peer dependency
+    if (macroCondition(dependencySatisfies('ember-engines', '*'))) {
+      // Use importSync for compile-time conditional import
+      const module = importSync(
         'ember-engines/components/link-to-external-component'
-      );
-      // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-      return mod.default as typeof LinkTo;
-    } catch {
+      ) as { default: typeof LinkTo };
+      return module.default;
+    } else {
       assert(
         `@isRouteExternal is only available when using the "ember-engines" addon. Please install it to use this feature.`,
         false

showcase/config/ember-try.js

@@ -21,6 +21,14 @@ module.exports = async function () {
           },
         },
       },
+      {
+        name: 'hds-ember',
+        npm: {
+          devDependencies: {
+            'ember-source': '~6.4.0',
+          },
+        },
+      },
       {
         name: 'ember-release',
         npm: {

showcase/tests/integration/components/hds/advanced-table/index-test.js

@@ -376,7 +376,8 @@ const hbsNestedAdvancedTable = hbs`<Hds::AdvancedTable
   </:body>
 </Hds::AdvancedTable>`;
 
-const hbsResizableColumnsAdvancedTable = hbs`<div style="width: 1000px;">
+const hbsResizableColumnsAdvancedTable = hbs`{{!-- template-lint-disable no-inline-styles --}}
+  <div style="width: 1000px;">
   <Hds::AdvancedTable
   @model={{this.model}} @columns={{this.columns}} @hasResizableColumns={{true}} id="resize-test-table"
 >
@@ -386,7 +387,7 @@ const hbsResizableColumnsAdvancedTable = hbs`<div style="width: 1000px;">
       <B.Td>{{B.data.col2}}</B.Td>
     </B.Tr>
   </:body>
-</Hds::AdvancedTable></div>`;
+</Hds::AdvancedTable></div>{{!-- template-lint-enable no-inline-styles --}}`;
 
 module('Integration | Component | hds/advanced-table/index', function (hooks) {
   setupRenderingTest(hooks);
@@ -727,10 +728,14 @@ module('Integration | Component | hds/advanced-table/index', function (hooks) {
       );
       await focus(firstThElement);
       await focus(firstReorderHandle);
+      // need to flush the frame to let the RAF waiter finish doing its thing
+      await new Promise((resolve) => requestAnimationFrame(resolve));
+
       assert.dom(firstReorderHandle).isFocused();
 
       await triggerKeyEvent(firstReorderHandle, 'keydown', 'ArrowRight');
       let columnOrder = await getColumnOrder(this.columns);
+      await settled();
       assert.deepEqual(
         columnOrder,
         [this.columns[1].key, this.columns[0].key, this.columns[2].key],
@@ -740,6 +745,8 @@ module('Integration | Component | hds/advanced-table/index', function (hooks) {
 
       await triggerKeyEvent(firstReorderHandle, 'keydown', 'ArrowRight');
       columnOrder = await getColumnOrder(this.columns);
+      // doing this because request animation frame stuff
+      await settled();
       assert.deepEqual(
         columnOrder,
         [this.columns[1].key, this.columns[2].key, this.columns[0].key],
@@ -749,6 +756,7 @@ module('Integration | Component | hds/advanced-table/index', function (hooks) {
 
       await triggerKeyEvent(firstReorderHandle, 'keydown', 'ArrowLeft');
       columnOrder = await getColumnOrder(this.columns);
+      await settled();
       assert.deepEqual(
         columnOrder,
         [this.columns[1].key, this.columns[0].key, this.columns[2].key],
@@ -842,7 +850,7 @@ module('Integration | Component | hds/advanced-table/index', function (hooks) {
       ];
 
       // when dealing with dynamic columns, you must handle the order of all potential columns rather than just the ones currently rendered
-      // inital column order is 'artist', 'album', 'year', 'genre'
+      // initial column order is 'artist', 'album', 'year', 'genre'
       const initialColumnOrder = availableColumns.map((col) => col.key);
 
       // initially set the columns in the reverse order to ensure the table respects the column order and ommit the genre column
@@ -876,6 +884,7 @@ module('Integration | Component | hds/advanced-table/index', function (hooks) {
 
       // make sure the initial column order is correct based on the columnOrder
       let columnOrder = await getColumnOrder(this.columns);
+      await settled();
       assert.deepEqual(
         columnOrder,
         ['artist', 'album', 'year'],

showcase/tests/integration/components/hds/dropdown/toggle/icon-test.js

@@ -3,14 +3,9 @@
  * SPDX-License-Identifier: MPL-2.0
  */
 
-import { module, skip, test } from 'qunit';
+import { module, test } from 'qunit';
 import { setupRenderingTest } from 'showcase/tests/helpers';
-import {
-  render,
-  resetOnerror,
-  settled,
-  setupOnerror,
-} from '@ember/test-helpers';
+import { render, resetOnerror, setupOnerror } from '@ember/test-helpers';
 import { hbs } from 'ember-cli-htmlbars';
 
 module('Integration | Component | hds/dropdown/toggle/icon', function (hooks) {
@@ -40,21 +35,17 @@ module('Integration | Component | hds/dropdown/toggle/icon', function (hooks) {
 
   // IMAGE (AVATAR)
 
-  test('if an @imageSrc is declared and exists the image should render in the component', async function (assert) {
+  test('if an @imageSrc is declared and also exists, the image should render in the component', async function (assert) {
     await render(
       hbs`<Hds::Dropdown::Toggle::Icon @icon="user" @text="user menu" @imageSrc="/assets/images/avatar.png" id="test-toggle-icon" />`,
     );
     assert.dom('img').exists();
   });
 
-  skip('if an @imageSrc is declared but does not exist, the flight icon should render in the component', async function (assert) {
-    this.set('imageSrc', '/assets/images/avatar.png');
+  test('if an @imageSrc is declared but the file does not exist, the flight icon should render in the component', async function (assert) {
     await render(
-      hbs`<Hds::Dropdown::Toggle::Icon @icon="user" @text="user menu" @imageSrc={{this.imageSrc}} id="test-toggle-icon" />`,
+      hbs`<Hds::Dropdown::Toggle::Icon @icon="user" @text="user menu" @imageSrc='/assets/images/avatar-broken.png' id="test-toggle-icon" />`,
     );
-    // we load the image dynamically to cover this usecase and also to prevent this test from intermittently failing for no obvious reason
-    this.set('imageSrc', '/assets/images/avatar-broken.png');
-    await settled();
     assert.dom('img').doesNotExist();
     assert.dom('#test-toggle-icon .hds-icon.hds-icon-user').exists();
   });

showcase/tests/integration/components/hds/popover-primitive/index-test.js

@@ -169,6 +169,7 @@ module(
         .dom('[data-test-id="popover-content"]')
         .isNotVisible('The popover is hidden again by the new toggle');
     });
+    // we have to skip this test because no CSS classes are applied to the popover primitive but I want to leave this test here so we don't think a test is missing.
     skip('it should toggle the popover visibility on click', async function (assert) {
       await render(hbs`
         <Hds::PopoverPrimitive @enableClickEvents={{true}}>
@@ -207,7 +208,7 @@ module(
           as |PP|
         >
           <div {{PP.setupPrimitiveContainer}}>
-            <button {{PP.setupPrimitiveToggle}}  id="test-popover-primitive-toggle" />
+            <button type="button" {{PP.setupPrimitiveToggle}}  id="test-popover-primitive-toggle" />
             <div {{PP.setupPrimitivePopover}} />
           </div>
         </Hds::PopoverPrimitive>

website/tests/acceptance/components/breadcrumb-test.js

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: MPL-2.0
  */
 
-import { module, test, skip } from 'qunit';
+import { module, test } from 'qunit';
 import { visit, currentURL } from '@ember/test-helpers';
 import { setupApplicationTest } from 'website/tests/helpers';
 import { a11yAudit } from 'ember-a11y-testing/test-support';
@@ -17,7 +17,7 @@ module('Acceptance | components/breadcrumb', function (hooks) {
     assert.strictEqual(currentURL(), '/components/breadcrumb');
   });
 
-  skip('components/breadcrumb page passes automated a11y checks', async function (assert) {
+  test('components/breadcrumb page passes automated a11y checks', async function (assert) {
     await visit('/components/breadcrumb');
     await a11yAudit();
     assert.ok(true, 'a11y automation audit passed');

website/tests/acceptance/components/tabs-test.js

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: MPL-2.0
  */
 
-import { module, test, skip } from 'qunit';
+import { module, test } from 'qunit';
 import { visit, currentURL } from '@ember/test-helpers';
 import { setupApplicationTest } from 'website/tests/helpers';
 import { a11yAudit } from 'ember-a11y-testing/test-support';
@@ -17,7 +17,7 @@ module('Acceptance | components/tabs', function (hooks) {
     assert.strictEqual(currentURL(), '/components/tabs');
   });
 
-  skip('components/tabs passes a11y automated checks', async function (assert) {
+  test('components/tabs passes a11y automated checks', async function (assert) {
     await visit('/components/tabs');
     await a11yAudit();
     assert.ok(true, 'a11y automation audit passed');