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.

Sign up for GitHub

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

Jump to bottom

Serialize Executor events #1054

Closed
wants to merge 6 commits into from
Closed

Serialize Executor events #1054

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7989044
Serialize Executor events
rhpijnacker Jan 19, 2020
c8a7a5a
Use separate notification chain for coverage, error and log events
rhpijnacker Mar 13, 2020
e476dd9
Small update
rhpijnacker Mar 13, 2020
715051f
Fix build error
rhpijnacker Mar 13, 2020
275e455
Trigger new build
rhpijnacker Mar 13, 2020
f9a99b5
IE10,IE11 do not like [].includes()
rhpijnacker Mar 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/lib/executors/Executor.ts
View file
Open in desktop
Expand Up @@ -92,6 +92,7 @@ export default abstract class BaseExecutor<
protected _loadingPlugins: { name: string; init: CancellablePromise<void> }[];
protected _loadingPluginOptions: any | undefined;
protected _listeners: { [event: string]: Listener<any>[] };
protected _notifications: CancellablePromise<void>;
protected _plugins: { [name: string]: any };
protected _reporters: Reporter[];
protected _runTask: CancellablePromise<void> | undefined;
Expand Down Expand Up @@ -144,6 +145,7 @@ export default abstract class BaseExecutor<
}

this._rootSuite = new Suite({ executor: this });
this._notifications = Task.resolve();

// This is the first suiteEnd listener. When the root unit test suite
// ends, it will emit a coverage message before any other suiteEnd
Expand Down Expand Up @@ -296,7 +298,12 @@ export default abstract class BaseExecutor<
}
};

let notifications = Task.resolve();
const needsSeparateNotificationChain =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of singling out coverage, error and log here and calling it needsSeparateNotificationChain, maybe we should name it after the actual events that are part of the test run. E.g. something like isTestLifeCycleEvent. Does that make sense?

['coverage', 'error', 'log'].indexOf(<string>eventName) > -1;

let notifications = needsSeparateNotificationChain
? Task.resolve()
: this._notifications;
let hasNotifications = false;

// First, notify the listeners specifically listening for this event
Expand Down Expand Up @@ -341,10 +348,11 @@ export default abstract class BaseExecutor<
} instead.`
);
}

return Task.resolve();
}

if (!needsSeparateNotificationChain) {
this._notifications = notifications;
}
return notifications;
}

Expand Down