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

Remove code that specifies different behaviour in tests inside of BasicDropdownWormhole #931

Closed
ArnaudWeyts opened this issue Jul 25, 2024 · 2 comments · Fixed by #937
Closed

Comments

@ArnaudWeyts
Copy link
Contributor

Why

Our visual tests render a duplicate #ember-testing container because of BasicDropdownWormhole

What

if (config.environment === 'test') {
// document doesn't exists in fastboot apps, for this reason we need this check
if (typeof document === 'undefined') {
return '';
}
const rootElement = config['APP']?.rootElement;
return document.querySelector(rootElement)?.id ?? '';
}

This part of the code will result in the following structure for visual tests:

<div id="ember-testing">
  <div id="ember-testing"></div>
</div>

I believe that this should actually be removed (except for the fastboot stuff possibly?), since visual tests use the application.hbs template, the wormhole should get rendered using the regular flow.

What might be required though is providing a test setup helper that does render the wormhole for regular rendering tests that don't use the application.hbs template.

What we've done for now as a workaround is not rendering the BasicDropdownWormhole in application.hbs in a testing environment and instead setting up the required HTML ourselves in a test helper:

import { getRootElement } from '@ember/test-helpers';

export function setupWormhole(hooks: NestedHooks) {
  hooks.beforeEach(function () {
    const wormholeBasicDropdown = document.createElement('div');
    wormholeBasicDropdown.id = 'ember-basic-dropdown-wormhole';
    getRootElement().appendChild(wormholeBasicDropdown);
  });

  hooks.afterEach(function () {
    const wormholeBasicDropdown = document.getElementById('ember-basic-dropdown-wormhole');
    wormholeBasicDropdown?.remove();
  });
}

This can then be setup just for rendering tests:

// app/tests/helpers/index.ts
import {
  setupRenderingTest as upstreamSetupRenderingTest,
} from 'ember-qunit';

function setupRenderingTest(hooks: NestedHooks, options?: SetupTestOptions) {
  upstreamSetupRenderingTest(hooks, options);
  setupWormhole(hooks);
}

Would love to hear your thoughts on this, and thanks for the work on this project!

@NullVoxPopuli
Copy link

just ran in to this

@ArnaudWeyts ArnaudWeyts changed the title Remove code that specify different behaviour in tests inside of BasicDropdownWormhole Remove code that specifies different behaviour in tests inside of BasicDropdownWormhole Aug 14, 2024
@mkszepp
Copy link
Collaborator

mkszepp commented Aug 27, 2024

fix released in v8.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants