Skip to content

Commit 2e9eb18

Browse files
authored
chore: migrate test runner from mocha to jest (#2567)
# Migrate Winston Test Suite from Mocha to Jest This PR fully migrates Winston's test suite from Mocha to Jest to provide a more fully featured testing framework > [!WARNING] > The File Transport tests while passing in their current form, illustrate in my opinion some potential issues in how it currently functions. > - You can see the `FIX:` comments on a couple of assertions that I presume are correct but are currently failing. > - The intersection of behavior with with the `maxsize` and `lazy` options result in behavior I found unexpected. e.g. If the rotation is not lazy, it will drop a log file resulting in 1 less populated log file than I would expect > - The maxsize option appears not to be strictly enforced on a per-file basis. This is probably desirable as in maxsize is likely on a best effort basis. I make this presumption given we wouldn't want to split a log payload across two files, nor do I think we should truncate it. The documentation for the Transport does not state either way though so look to someone with more familiarity than I about original intent.d ## Summary - Refactored failing tests to be Jest-compatible - Primarily this was just modify `before()`, `after()`, `this.timeout()` to `beforeAll()`, `afterAll()`, and `jest.setTimeout()` - Moved the `winston.test.js` from `integration` to `unit`. This felt more appropriate given the contents of the test. - Refactored some tests for clarity or to resolve testing framework incompatabilities - Jest intercepts all stderr and pipes to stdout, so some tests have been updated to rely on Spies of `console.error` - Introduce async/await in some tests to reduce function nesting. - Combine File Archive tests and remove tests that I felt were redundant. This ensures these tests run sequentially as they're in the same file. This helps to avoid the potential of race conditions. Further improvements could likely be made here. Lastly these are not what I would label as unit tests and would likely advocate moving them to the `integration` suite given they're actually writing files to the FS. ## Test Plan - All tests are passing in CI environment - Manual verification of test coverage shows equivalent or better coverage compared to Mocha - Verified file system tests properly clean up temporary files - Confirmed logging tests produce expected output formats
1 parent 4dc03d6 commit 2e9eb18

38 files changed

+8646
-8271
lines changed

.eslintrc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
{
2+
"env": {
3+
"jest": true,
4+
},
25
"extends": "@dabh/eslint-config-populist",
36
"rules": {
47
"one-var": ["error", { "var": "never", "let": "never", "const": "never" }],

.github/workflows/ci.yml

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ jobs:
2121

2222
runs-on: ubuntu-latest
2323
strategy:
24+
fail-fast: false
2425
matrix:
2526
node:
2627
- 16
@@ -31,18 +32,24 @@ jobs:
3132
- uses: actions/setup-node@v4
3233
with:
3334
node-version: ${{ matrix.node }}
35+
3436
- name: Install Dependencies
3537
run: npm clean-install
38+
3639
- name: Lint
3740
run: npm run lint
41+
42+
- name: Unit Tests (with coverage)
43+
run: npm run test:unit
44+
3845
- name: Integration Tests
3946
run: npm run test:integration
40-
- name: Test Coverage
41-
run: npm run test:coverage
47+
4248
- name: Report test coverage to Coveralls.io
4349
if: matrix.node == '20'
4450
uses: coverallsapp/github-action@master
4551
with:
4652
github-token: ${{ secrets.GITHUB_TOKEN }}
53+
4754
- name: TypeScript Test
48-
run: npx --package typescript tsc --project test
55+
run: npm run test:typescript

.mocharc.yml

Lines changed: 0 additions & 5 deletions
This file was deleted.

.nycrc.yml

Lines changed: 0 additions & 8 deletions
This file was deleted.

README.md

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,15 +1234,17 @@ yarn add winston
12341234

12351235
## Run Tests
12361236

1237-
All of the winston tests are written with [`mocha`][mocha], [`nyc`][nyc], and
1238-
[`assume`][assume]. They can be run with `npm`.
1239-
12401237
``` bash
1241-
npm test
1238+
npm test # Runs all tests
1239+
npm run test:unit # Runs all Unit tests with coverage
1240+
npm run test:integration # Runs all integration tests
1241+
npm run test:typescript # Runs tests verifying Typescript types
12421242
```
12431243

1244+
All of the winston tests are written with [jest]. Assertions use a mix of [assume] and the built-in jest assertion library.
1245+
12441246
#### Author: [Charlie Robbins]
1245-
#### Contributors: [Jarrett Cruger], [David Hyde], [Chris Alderson]
1247+
#### Contributors: [Jarrett Cruger], [David Hyde], [Chris Alderson], [Jonathon Terry]
12461248

12471249
[Transports]: #transports
12481250
[Logging levels]: #logging-levels
@@ -1254,11 +1256,10 @@ npm test
12541256

12551257
[RFC5424]: https://tools.ietf.org/html/rfc5424
12561258
[util.format]: https://nodejs.org/dist/latest/docs/api/util.html#util_util_format_format_args
1257-
[mocha]: https://mochajs.org
1258-
[nyc]: https://github.com/istanbuljs/nyc
12591259
[assume]: https://github.com/bigpipe/assume
12601260
[logform]: https://github.com/winstonjs/logform#readme
12611261
[winston-transport]: https://github.com/winstonjs/winston-transport
1262+
[jest]: https://jestjs.io/
12621263

12631264
[Read the `[email protected]` documentation]: https://github.com/winstonjs/winston/tree/2.x
12641265

@@ -1268,4 +1269,5 @@ npm test
12681269
[Charlie Robbins]: https://github.com/indexzero
12691270
[Jarrett Cruger]: https://github.com/jcrugzz
12701271
[David Hyde]: https://github.com/dabh
1271-
[Chris Alderson]: https://github.com/chrisalderson
1272+
[Chris Alderson]: https://github.com/chrisalderson
1273+
[Jonathon Terry]: https://github.com/maverick1872

jest.config.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* @type {import('@jest/types').Config.InitialOptions}
3+
*/
4+
module.exports = {
5+
collectCoverage: false,
6+
collectCoverageFrom: [
7+
'<rootDir>/lib/**/*.js'
8+
],
9+
coverageDirectory: 'coverage',
10+
testEnvironment: 'node',
11+
testMatch: [
12+
'<rootDir>/test/**/*.test.js'
13+
],
14+
globalSetup: '<rootDir>/test/globalSetup.js',
15+
silent: true,
16+
verbose: true,
17+
coverageThreshold: {
18+
global: {
19+
functions: 74.54,
20+
lines: 72.48,
21+
statements: 72.25,
22+
branches: 64.04
23+
}
24+
}
25+
};
26+

lib/winston/profiler.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,6 @@ class Profiler {
4848

4949
return this.logger.write(info);
5050
}
51-
};
51+
}
5252

5353
module.exports = Profiler;

0 commit comments

Comments
 (0)