Skip to content

[tests] ensure that the notifier is emptied by each test #8822

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

Merged
merged 2 commits into from
Oct 25, 2016

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Oct 25, 2016

While working on #8734 I noticed that:

  • we were not clearing the notifier after tests put notifications in them
  • when errors ended up in the notifier unexpectedly tests would still pass

This fixes these issues by checking before each test run that notifications were properly cleared out after the last test.

@jbudz
Copy link
Member

jbudz commented Oct 25, 2016

LGTM

Copy link
Contributor

@Bargs Bargs left a comment

Choose a reason for hiding this comment

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

One minor comment, otherwise LGTM

beforeEach(function () {
if (Notifier.prototype._notifs.length) {
Notifier.prototype._notifs.length = 0;
throw new TypeError('notifications were left in the notifier');
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem like a TypeError to me

Copy link
Contributor Author

@spalger spalger Oct 25, 2016

Choose a reason for hiding this comment

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

habit :)

@spalger spalger merged commit 973222c into elastic:master Oct 25, 2016
elastic-jasper added a commit that referenced this pull request Oct 25, 2016
---------

**Commit 1:**
[tests] ensure that the notifier is emptied by each test

* Original sha: 0cdef8c
* Authored by spalger <[email protected]> on 2016-10-25T00:09:57Z

**Commit 2:**
[testHarness] use generic Error class

* Original sha: 34659fb
* Authored by spalger <[email protected]> on 2016-10-25T22:11:39Z
spalger added a commit that referenced this pull request Oct 25, 2016
[backport] PR #8822 to 5.x - [tests] ensure that the notifier is emptied by each test
@epixa epixa added v5.1.1 and removed v5.1.0 labels Dec 8, 2016
airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017
---------

**Commit 1:**
[tests] ensure that the notifier is emptied by each test

* Original sha: 11e9af0d765960bcf586bbebe2d9309b1506d264 [formerly 0cdef8c]
* Authored by spalger <[email protected]> on 2016-10-25T00:09:57Z

**Commit 2:**
[testHarness] use generic Error class

* Original sha: d7e994e720ebdb6a2c0cd499936956ad1fe1fbee [formerly 34659fb]
* Authored by spalger <[email protected]> on 2016-10-25T22:11:39Z


Former-commit-id: cfb469b
airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017
[backport] PR elastic#8822 to 5.x - [tests] ensure that the notifier is emptied by each test

Former-commit-id: 9e7afe5
@spalger spalger deleted the fix/tests/ensure-empty-notifs branch October 18, 2019 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants