Skip to content
This repository was archived by the owner on Apr 21, 2022. It is now read-only.

Fix the tests so they don't fail with the recent updates#82

Merged
smusali merged 5 commits intomasterfrom
fixFlushAllCallback
Nov 15, 2019
Merged

Fix the tests so they don't fail with the recent updates#82
smusali merged 5 commits intomasterfrom
fixFlushAllCallback

Conversation

@vilyapilya
Copy link
Copy Markdown
Contributor

#76 - changes are good but they break the current tests.
The tests fail bc the code continues to execute after done is called in aftereach block. The initial intention of the block was to clean the logger after each test. I removed afterEach part. It is not needed anymore since each test creates its own logger.

Copy link
Copy Markdown
Contributor

@smusali smusali left a comment

Choose a reason for hiding this comment

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

@vilyapilya, please, make sure it passes the linting check

@smusali smusali changed the title Fix the tests so they don't fail with the updates in https://github.com/logdna/nodejs/pull/76 Fix the tests so they don't fail with the recent updates Nov 8, 2019
guidolodetti and others added 5 commits November 8, 2019 11:09
@smusali smusali self-requested a review November 11, 2019 17:33
Comment thread lib/logger.js
const errors = [];
loggers.forEach((logger) => {
logger._flush((err) => {
if (err) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use error for readability since the array is errors - basically, adding error into errors: the list of errors

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think it makes a difference.

@smusali smusali merged commit 73b9c71 into master Nov 15, 2019
@smusali smusali deleted the fixFlushAllCallback branch November 15, 2019 21:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants