Skip to content

Commit 0934011

Browse files
devversiondylhunn
authored andcommitted
test: avoid test fixture affecting zone in all web tests (angular#46511)
We have a file called `test-events.js` (named in an ambiguous way anyway) that runs for all Karma web tests and configures ZoneJS to not patch the `scroll` event. There are two issues: 1. The patch applies to all web tests. This could cause unexpected issues. 2. The file is named ambiguously and also is placed at the project root, in a wrong spot. Additionally, the test doesn't even fail when the file is removed. This commit applies the Zone config locally to the closest build target and also reworks the test to actually ensure it's testing what it describes. PR Close angular#46511
1 parent d4c10d1 commit 0934011

File tree

7 files changed

+51
-28
lines changed

7 files changed

+51
-28
lines changed

BUILD.bazel

-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ filegroup(
2525
"//packages/zone.js/bundles:zone.umd.js",
2626
"//packages/zone.js/bundles:zone-testing.umd.js",
2727
"//packages/zone.js/bundles:task-tracking.umd.js",
28-
"//:test-events.js",
2928
# Including systemjs because it defines `__eval`, which produces correct stack traces.
3029
"@npm//:node_modules/systemjs/dist/system.src.js",
3130
"@npm//:node_modules/reflect-metadata/Reflect.js",

karma-js.conf.js

+4
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ module.exports = function(config) {
3636
'node_modules/core-js-bundle/index.js',
3737
'node_modules/jasmine-ajax/lib/mock-ajax.js',
3838

39+
// ZoneJS configuration needed for some event manager tests. This config could
40+
// affect all legacy tests but in reality is scoped to certain special tests.
41+
'packages/platform-browser/test/dom/events/zone_event_unpatched_init.js',
42+
3943
// Dependencies built by Bazel. See `config.yml` for steps running before
4044
// the legacy Saucelabs tests run.
4145
'dist/bin/packages/zone.js/npm_package/bundles/zone.umd.js',

packages/platform-browser/test/BUILD.bazel

+4
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ jasmine_node_test(
5050

5151
karma_web_test_suite(
5252
name = "test_web",
53+
bootstrap = [
54+
"dom/events/zone_event_unpatched_init.js",
55+
"//:web_test_bootstrap_scripts",
56+
],
5357
static_files = [
5458
":static_assets/test.html",
5559
],

packages/platform-browser/test/dom/events/event_manager_spec.ts

+25-17
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {ɵgetDOM as getDOM} from '@angular/common';
1010
import {NgZone} from '@angular/core/src/zone/ng_zone';
1111
import {DomEventsPlugin} from '@angular/platform-browser/src/dom/events/dom_events';
1212
import {EventManager, EventManagerPlugin} from '@angular/platform-browser/src/dom/events/event_manager';
13+
1314
import {createMouseEvent, el} from '../../../testing/src/browser_util';
1415

1516
(function() {
@@ -310,28 +311,35 @@ describe('EventManager', () => {
310311
expect(receivedEvents).toEqual([]);
311312
});
312313

314+
// This test is reliant on `zone_event_unpatched_init.js` and verifies
315+
// that the Zone unpatched event setting applies to the event manager.
313316
it('should run unpatchedEvents handler outside of ngZone', () => {
314-
const Zone = (window as any)['Zone'];
315317
const element = el('<div><div></div></div>');
318+
const zone = new NgZone({enableLongStackTrace: true});
319+
const manager = new EventManager([domEventPlugin], zone);
320+
let timeoutId: NodeJS.Timeout|null = null;
321+
316322
doc.body.appendChild(element);
317-
const dispatchedEvent = createMouseEvent('scroll');
318-
let receivedEvent: any /** TODO #9100 */ = null;
319-
let receivedZone: any = null;
320-
const handler = (e: any /** TODO #9100 */) => {
321-
receivedEvent = e;
322-
receivedZone = Zone.current;
323-
};
324-
const manager = new EventManager([domEventPlugin], new FakeNgZone());
325323

326-
let remover = manager.addEventListener(element, 'scroll', handler);
327-
getDOM().dispatchEvent(element, dispatchedEvent);
328-
expect(receivedEvent).toBe(dispatchedEvent);
329-
expect(receivedZone.name).not.toEqual('angular');
324+
// Register the event listener in the Angular zone. If the handler would be
325+
// patched then, the Zone should propagate into the listener callback.
326+
zone.run(() => {
327+
manager.addEventListener(element, 'unpatchedEventManagerTest', () => {
328+
// schedule some timer that would cause the zone to become unstable. if the event
329+
// handler would be patched, `hasPendingMacrotasks` would be `true`.
330+
timeoutId = setTimeout(() => {}, 9999999);
331+
});
332+
});
330333

331-
receivedEvent = null;
332-
remover && remover();
333-
getDOM().dispatchEvent(element, dispatchedEvent);
334-
expect(receivedEvent).toBe(null);
334+
expect(zone.hasPendingMacrotasks).toBe(false);
335+
getDOM().dispatchEvent(element, createMouseEvent('unpatchedEventManagerTest'));
336+
337+
expect(zone.hasPendingMacrotasks).toBe(false);
338+
expect(timeoutId).not.toBe(null);
339+
340+
// cleanup the DOM by removing the test element we attached earlier.
341+
doc.body.removeChild(element);
342+
timeoutId && clearTimeout(timeoutId);
335343
});
336344

337345
it('should only trigger one Change detection when bubbling with shouldCoalesceEventChangeDetection = true',
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
// NOTE: This file affects most of tests in `platform-browser/test`. Make sure to
10+
// not change semantics of Angular that would result in false-positives. If you need
11+
// to change semantics of Angular, please create a separate test BUILD target.
12+
13+
// This file marks the `unpatchedEventManagerTest` event as unpatched. This is not
14+
// strictly needed, but done for a specific test verifying that unpatched events are
15+
// running outside the zone. See `event_manager_spec.ts` and the
16+
// `unpatchedEvents handler outside of ngZone` spec.
17+
window.__zone_symbol__UNPATCHED_EVENTS = ['unpatchedEventManagerTest'];

packages/zone.js/lib/common/events.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ export function patchEventTarget(
396396
const options = buildEventListenerOptions(arguments[2], passive);
397397

398398
if (unpatchedEvents) {
399-
// check upatched list
399+
// check unpatched list
400400
for (let i = 0; i < unpatchedEvents.length; i++) {
401401
if (eventName === unpatchedEvents[i]) {
402402
if (passive) {

test-events.js

-9
This file was deleted.

0 commit comments

Comments
 (0)