emberjs / ember-test-helpers

Test-framework-agnostic helpers for testing Ember.js applications
Apache License 2.0
188 stars 254 forks source link

Proposal: Allow context to teardown and setup repeatedly in a test #1452

Open mixonic opened 1 month ago

mixonic commented 1 month ago

The goal

I maintain a suite of tests that use a helper simulatePageRefresh to destroy the currently running application, then instantiate a new one. The implementation, compatible until ember-qunit 4 (and possibly 5), looks like this:

export async function simulatePageRefresh() {
  let context = getContext();

  await teardownContext(context);
  await setupContext(context);
  await setupApplicationContext(context);
}

This helper allows us to assert, in application tests, that a fresh page load (from Ember's perspective) performs data loading correctly (either from mocked APIs or from real APIs).

This functionality is not a robust as a true page reload, such as we would get from writing these tests in Playwright. However, as we already have these tests in Ember-QUnit and Ember-QUnit already embraces booting an application multiple times in the same browsing context, this functionality doesn't seem bizarre.

Why our implementation no longer works

In ember-qunit, setupContext and teardownContext define how a testing context is configured and reset. See https://github.com/emberjs/ember-qunit/blob/dafee6bdb9a93ab6a83de7421adad6669df31d95/addon/src/index.js#L51

setupContext, at https://github.com/emberjs/ember-test-helpers/blob/9cec68dc6aa9c0a7a449eb89797eb81299fa727f/addon/addon-test-support/%40ember/test-helpers/setup-context.ts#L382 uses the QUnit test context (passed here) as a key for destructors and even adds the context into the destroyable tree (associated with the Ember owner instance) at https://github.com/emberjs/ember-test-helpers/blob/9cec68dc6aa9c0a7a449eb89797eb81299fa727f/addon/addon-test-support/%40ember/test-helpers/setup-context.ts#L425.

Although teardownContext calls destroy with the context at https://github.com/emberjs/ember-test-helpers/blob/9cec68dc6aa9c0a7a449eb89797eb81299fa727f/addon/addon-test-support/%40ember/test-helpers/teardown-context.ts#L25, this does not prevent the context from being tracked as "destroyed". That state then makes it impossible to call setupContext a second time, as the destructible codepaths complain the instance was already destroyed.

Proposal for discussion

I think there are several ways forward:

This issue is to gather feedback from others and ideally gain some consensus before we do the technical implementation.

cc @eliasdawson-addepar @NullVoxPopuli @IgnaceMaes

NullVoxPopuli commented 1 month ago

I think if this library can figure out any way to simulate a refresh, that's fantastic, and I'd be very happy to see such a feature -- I've implemented something like what you have at a prior employer, and it was a good tool.

I don't think it makes too much sense to have a first-class helper for "restarting the application" yet, because it could require figuring out how to clean up DOM body/head/etc. Since test-helpers renders things into the test container, via ember-qunit, I think simulating a refresh is much safer in library land.