Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 0 additions & 78 deletions addon/src/-internal/-owner-mixin-imports.ts

This file was deleted.

14 changes: 5 additions & 9 deletions addon/src/-internal/build-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,15 @@ import type { Resolver } from '@ember/owner';
import ApplicationInstance from '@ember/application/instance';
import Application from '@ember/application';
import EmberObject from '@ember/object';

import Ember from 'ember';
import { Registry } from '@ember/-internals/container';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we specified these with the barrel deprecation RFC, but I wonder if there is anything in ember-source testing to make sure we don't break such import paths?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they're all private API, so I think it's totally fair for ember to break them if it wishes.

@ember/test-helpers should not be using private APIs haha

(we can explore migrate off of them in a separate PR)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They may be private but if we don't do that follow up PR all the pain will be ours.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aye, I'm doing this now 🎉

import { ComponentLookup } from '@ember/-internals/views';

import type { FullName } from '@ember/owner';

// These shenanigans work around the fact that the import locations are not
// public API and are not stable, so we jump through hoops to get the right
// types and values to use.
import {
ContainerProxyMixin,
RegistryProxyMixin,
} from './-owner-mixin-imports.ts';
} from '@ember/-internals/runtime';

/**
* Adds methods that are normally only on registry to the container. This is largely to support the legacy APIs
Expand Down Expand Up @@ -102,10 +99,9 @@ export default function buildRegistry(resolver: Resolver) {
const fallbackRegistry = Application.buildRegistry(namespace);
// TODO: only do this on Ember < 3.13
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment relevant? If so, do we even need to be supporting Ember < 3.13 here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't, but I'd argue cleanup for legacy stuff should be in a different PR (which I can submit next! as there are a number of other cleanup opportunities, I think)

// @ts-ignore: this is private API.
fallbackRegistry.register('component-lookup:main', Ember.ComponentLookup);
fallbackRegistry.register('component-lookup:main', ComponentLookup);

// @ts-ignore: this is private API.
const registry = new Ember.Registry({
const registry = new Registry({
fallback: fallbackRegistry,
});

Expand Down
4 changes: 2 additions & 2 deletions addon/src/setup-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { setOwner } from '@ember/application';
import buildOwner, { type Owner } from './build-owner.ts';
import { _setupAJAXHooks } from './settled.ts';
import { _prepareOnerror } from './setup-onerror.ts';
import Ember from 'ember';
import { setTesting } from '@ember/debug';
import {
assert,
registerDeprecationHandler,
Expand Down Expand Up @@ -379,7 +379,7 @@ export default function setupContext<T extends object>(
const context = base as T & TestContext;

// SAFETY: this is intimate API *designed* for us to override.
(Ember as any).testing = true;
setTesting(true);
setContext(context);

const testMetadata = getTestMetadata(context);
Expand Down
9 changes: 5 additions & 4 deletions addon/src/setup-onerror.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Ember from 'ember';
import { getOnerror, setOnerror } from '@ember/-internals/error-handling';
import { type BaseContext, getContext } from './setup-context.ts';

const cachedOnerror: Map<BaseContext, ((error: Error) => void) | undefined> =
Expand Down Expand Up @@ -39,7 +39,7 @@ export default function setupOnerror(onError?: (error: Error) => void): void {
onError = cachedOnerror.get(context);
}

Ember.onerror = onError;
setOnerror(onError);
}

/**
Expand All @@ -60,7 +60,7 @@ export function resetOnerror(): void {
const context = getContext();

if (context && cachedOnerror.has(context)) {
Ember.onerror = cachedOnerror.get(context);
setOnerror(cachedOnerror.get(context));
}
}

Expand All @@ -76,7 +76,8 @@ export function _prepareOnerror(context: BaseContext) {
throw new Error('_prepareOnerror should only be called once per-context');
}

cachedOnerror.set(context, Ember.onerror);
// SAFETY: getOnerror doesn't have correct types
cachedOnerror.set(context, getOnerror() as any);
}

/**
Expand Down
7 changes: 3 additions & 4 deletions addon/src/setup-rendering-context.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { run } from '@ember/runloop';
import Ember from 'ember';
import { EventDispatcher } from '@ember/-internals/views';
import global from './global.ts';
import {
type BaseContext,
Expand Down Expand Up @@ -265,9 +265,8 @@ export default function setupRenderingContext(
// manually start the event dispatcher.
if (owner._emberTestHelpersMockOwner) {
const dispatcher =
owner.lookup('event_dispatcher:main') ||
(Ember.EventDispatcher as any).create();
dispatcher.setup({}, '#ember-testing');
owner.lookup('event_dispatcher:main') || EventDispatcher.create();
(dispatcher as any).setup({}, '#ember-testing');
}

const OutletView = owner.factoryFor
Expand Down
8 changes: 2 additions & 6 deletions addon/src/teardown-context.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { TestContext } from './setup-context';
import Ember from 'ember';
import { setTesting } from '@ember/debug';
import { unsetContext } from './setup-context.ts';
import settled, { _teardownAJAXHooks } from './settled.ts';
import { _cleanupOnerror } from './setup-onerror.ts';
Expand Down Expand Up @@ -31,14 +31,10 @@ export default function teardownContext(
return Promise.resolve()
.then(() => {
_cleanupOnerror(context);

_teardownAJAXHooks();

// SAFETY: this is intimate API *designed* for us to override.
(Ember as any).testing = false;

setTesting(false);
unsetContext();

destroy(context.owner);
})
.finally(() => {
Expand Down
13 changes: 8 additions & 5 deletions addon/src/validate-error-handler.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import Ember from 'ember';
// Private API
import { getOnerror } from '@ember/-internals/error-handling';
// Private API
import { isTesting, setTesting } from '@ember/debug';

type ErrorHandlerValidation =
| Readonly<{ isValid: true; message: null }>
Expand Down Expand Up @@ -34,24 +37,24 @@ const INVALID = Object.freeze({
* });
*/
export default function validateErrorHandler(
callback = Ember.onerror,
callback = getOnerror(),
): ErrorHandlerValidation {
if (callback === undefined || callback === null) {
return VALID;
}

const error = new Error('Error handler validation error!');

const originalEmberTesting = Ember.testing;
(Ember as any).testing = true;
const originalEmberTesting = isTesting();
setTesting(true);
try {
callback(error);
} catch (e) {
if (e === error) {
return VALID;
}
} finally {
(Ember as any).testing = originalEmberTesting;
setTesting(originalEmberTesting);
}

return INVALID;
Expand Down
1 change: 1 addition & 0 deletions test-app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
"pretender": "^3.4.7",
"prettier": "^2.8.7",
"qunit": "^2.21.1",
"qunit-console-grouper": "^0.3.0",
"qunit-dom": "^3.2.0",
"tracked-built-ins": "^3.1.1",
"webpack": "^5.78.0"
Expand Down
13 changes: 13 additions & 0 deletions test-app/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions test-app/tests/helpers/module-for-acceptance.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
import { resolve } from 'rsvp';
import { setTesting } from '@ember/debug';
import { module } from 'qunit';
import startApp from '../helpers/start-app';
import destroyApp from '../helpers/destroy-app';
import { setResolverRegistry } from './resolver';
import QUnitTestAdapter from './qunit-test-adapter';
import Ember from 'ember';
import { setAdapter } from 'ember-testing/lib/setup_for_testing';

export default function (name, options = {}) {
module(name, {
beforeEach() {
Ember.Test.adapter = QUnitTestAdapter.create();
setAdapter(QUnitTestAdapter.create());

if (options.registry) {
setResolverRegistry(options.registry);
Expand All @@ -20,7 +21,7 @@ export default function (name, options = {}) {
);
this.fixtureResetValue = testElementContainer.innerHTML;

Ember.testing = true;
setTesting(true);
this.application = startApp();

if (options.beforeEach) {
Expand All @@ -34,7 +35,7 @@ export default function (name, options = {}) {
return resolve(afterEach)
.then(() => destroyApp(this.application))
.finally(() => {
Ember.testing = false;
setTesting(false);

document.getElementById('ember-testing-container').innerHTML =
this.fixtureResetValue;
Expand Down
19 changes: 10 additions & 9 deletions test-app/tests/helpers/qunit-module-for.js
Original file line number Diff line number Diff line change
@@ -1,29 +1,30 @@
import Ember from 'ember';
import { isTesting } from '@ember/debug';
import { module } from 'qunit';
import QUnitTestAdapter from './qunit-test-adapter';
import { setAdapter } from 'ember-testing/lib/setup_for_testing';

export default function qunitModuleFor(testModule) {
module(testModule.name, {
beforeEach(assert) {
if (Ember.testing) {
throw new Error('should not have Ember.testing === true in beforeEach');
if (isTesting()) {
throw new Error('should not have isTesting() === true in beforeEach');
}
Ember.Test.adapter = QUnitTestAdapter.create();
setAdapter(QUnitTestAdapter.create());
testModule.setContext(this);
return testModule.setup(assert).finally(() => {
if (!Ember.testing) {
if (!isTesting()) {
throw new Error(
'should have Ember.testing === true after tests have started'
'should have isTesting() === true after tests have started'
);
}
});
},
afterEach(assert) {
return testModule.teardown(assert).finally(() => {
Ember.Test.adapter = null;
if (Ember.testing) {
setAdapter(null);
if (isTesting()) {
throw new Error(
'should not have Ember.testing === true after tests have finished'
'should not have isTesting() === true after tests have finished'
);
}
});
Expand Down
4 changes: 2 additions & 2 deletions test-app/tests/helpers/qunit-test-adapter.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Ember from 'ember';
import QUnit from 'qunit';
import { Adapter } from '@ember/test';

/*
* This module is inlined here, because this library cannot depend on
Expand Down Expand Up @@ -30,7 +30,7 @@ function unhandledRejectionAssertion(current, error) {
});
}

export default Ember.Test.Adapter.extend({
export default Adapter.extend({
init() {
this.doneCallbacks = [];
},
Expand Down
Loading