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.

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

Chore: Organize and restructure tests #2049

Merged

Conversation

maverick1872
Copy link
Member

@maverick1872 maverick1872 commented Jan 30, 2022

Summary

This PR is a subset of the changes I've made on the #1989 PR. This is primarily a reorganization effort of Winston's tests to further clarify their intent and make more obvious what is being tested by what. I'm pretty positive there are still integration tests in the unit tests directory but it's at least moving things to be more explicit in nature.

Tech Notes (pre-emptive comments left on git diff)

  • Some cleanup has been applied to the path.join usage throughout all tests. These changes were necessary due to the directory level segregation of tests. While updating them I leveraged top-level constants to make them easier to update down the line.
  • Major re-organization to the logger.test.ts has been done. This was done to make the relationship between different tests more apparent.
  • Coverage is only ran against unit tests.
  • CI runs both unit test coverage and integration tests explicitly.
  • TODO's added for tests that leverage a vows depedency that is no longer present.
  • Introduced a .nycrc file for test's to retain a minimum level of coverage. This will help to ensure as new features are added the test coverage not continually degrade. I'd like to automate updating these values at some point but this is low priority.

package.json Outdated Show resolved Hide resolved
test/unit/winston/create-logger.test.js Show resolved Hide resolved
test/unit/winston/transports/console.test.js Show resolved Hide resolved
test/unit/winston/transports/file-create-dir.test.js Outdated Show resolved Hide resolved
Copy link
Contributor

@fearphage fearphage left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I had a few questions and suggestions.

.nycrc Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
test/helpers/scripts/default-exceptions.js Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
package.json Show resolved Hide resolved
test/unit/winston/transports/console.test.js Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
test/helpers/scripts/default-exceptions.js Show resolved Hide resolved
Copy link
Contributor

@fearphage fearphage left a comment

Choose a reason for hiding this comment

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

A few finishing touches

Copy link
Contributor

@wbt wbt left a comment

Choose a reason for hiding this comment

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

I'm not able to give this an in-depth review at this time, so I defer to those who can.

package.json Outdated Show resolved Hide resolved
@DABH
Copy link
Contributor

DABH commented Feb 5, 2022

FWIW I'm okay with this once @fearphage gives the +1 :)

@maverick1872 maverick1872 merged commit 2b8cd55 into winstonjs:master Feb 7, 2022
@maverick1872 maverick1872 deleted the chore/organize-logger-tests branch February 7, 2022 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants