Skip to content

test: unit test fails when using after msw 2.6.x #187

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

Merged
merged 5 commits into from
Nov 18, 2024
Merged

test: unit test fails when using after msw 2.6.x #187

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
461ec57
build(deps): update all non-major dependencies
renovate[bot] Nov 12, 2024
2099af0
fix: test fail because cross-fetch outdated & mswjs changed interface
duyhungtnn Nov 18, 2024
8b9bd9b
chore: remove comment code
duyhungtnn Nov 18, 2024
be3d641
chore: restore onUnhandledRequest options
duyhungtnn Nov 18, 2024
55e737d
chore: remove cross-fetch
duyhungtnn Nov 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .node-version
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
22.7.0
22.11.0
4 changes: 2 additions & 2 deletions example/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"@bucketeer/js-client-sdk": "workspace:*"
},
"devDependencies": {
"vite": "5.4.6",
"typescript": "5.5.4"
"vite": "5.4.11",
"typescript": "5.6.3"
}
}
18 changes: 9 additions & 9 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,25 +42,25 @@
},
"devDependencies": {
"@types/jsdom": "21.1.7",
"@types/node": "20.16.3",
"@typescript-eslint/eslint-plugin": "8.4.0",
"@typescript-eslint/parser": "8.4.0",
"@types/node": "20.17.6",
"@typescript-eslint/eslint-plugin": "8.14.0",
"@typescript-eslint/parser": "8.14.0",
"@vitest/browser": "2.1.4",
"@vitest/utils": "2.1.4",
"cross-fetch": "4.0.0",
"eslint": "9.9.1",
"eslint": "9.14.0",
"eslint-config-prettier": "9.1.0",
"eslint-plugin-import": "2.30.0",
"eslint-plugin-import": "2.31.0",
"eslint-plugin-prettier": "5.2.1",
"happy-dom": "15.7.3",
"jiti": "1.21.6",
"msw": "2.4.1",
"msw": "2.6.4",
"prettier": "3.3.3",
"typescript": "5.5.4",
"typescript-eslint": "8.4.0",
"typescript": "5.6.3",
"typescript-eslint": "8.14.0",
"unbuild": "2.0.0",
"vitest": "2.1.4",
"webdriverio": "9.2.12"
},
"packageManager": "pnpm@9.9.0"
"packageManager": "pnpm@9.12.3"
}
587 changes: 250 additions & 337 deletions pnpm-lock.yaml
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion test/BKTClient.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
TestPlatformModule,
FakeClock,
} from './utils'
import fetch from 'cross-fetch'
// import fetch from 'cross-fetch'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cross-fetch is outdated and isn't getting any update more than a years.

since Fetch is widely available (at least from Node.js 18 and higher).
so we could get rid of "cross-fetch" as a dependency.

I am testing more to make sure. So far all tests passed when run on both browser & node environment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tested with node 18 and higher and it seems good.

mswjs example for vitest could run on both browser & node environment without cross-fetch
https://github.com/mswjs/examples/tree/main/examples/with-vitest

import { user1 } from './mocks/users'
import {
InternalServerErrorException,
Expand Down
2 changes: 1 addition & 1 deletion test/internal/event/EventInteractor.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import assert from 'assert'
import { HttpResponse, http } from 'msw'
import { SetupServer } from 'msw/node'
import fetch from 'cross-fetch'
// import fetch from 'cross-fetch'
import {
expect,
suite,
Expand Down
7 changes: 2 additions & 5 deletions test/internal/remote/ApiClient.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
beforeAll,
vi,
} from 'vitest'
import fetch from 'cross-fetch'
// import fetch from 'cross-fetch'
import { HttpResponse, http, delay, StrictRequest } from 'msw'
import assert from 'assert'
import { SetupServer } from 'msw/node'
Expand Down Expand Up @@ -52,10 +52,7 @@ suite('internal/remote/ApiClient', () => {

suite('getEvaluations', () => {
test('success', async () => {
const requestInterceptor = vi.fn<
[StrictRequest<GetEvaluationsRequest>],
void
>()
const requestInterceptor = vi.fn((_request: StrictRequest<GetEvaluationsRequest>) => {})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

requestInterceptor should look like this with new mswjs version


server.use(
http.post<
Expand Down
2 changes: 1 addition & 1 deletion test/internal/scheduler/EvaluationTask.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
afterAll,
beforeAll,
} from 'vitest'
import fetch from 'cross-fetch'
// import fetch from 'cross-fetch'
import { destroyBKTClient } from '../../../src/BKTClient'
import { BKTConfig, defineBKTConfig } from '../../../src/BKTConfig'
import { DefaultComponent } from '../../../src/internal/di/Component'
Expand Down
2 changes: 1 addition & 1 deletion test/internal/scheduler/EventTask.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
afterAll,
describe,
} from 'vitest'
import fetch from 'cross-fetch'
// import fetch from 'cross-fetch'
import { destroyBKTClient } from '../../../src/BKTClient'
import { BKTConfig, defineBKTConfig } from '../../../src/BKTConfig'
import { DefaultComponent } from '../../../src/internal/di/Component'
Expand Down
2 changes: 1 addition & 1 deletion test/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export function setupServerAndListen(
...handlers: Array<RequestHandler>
): SetupServer {
const server = setupServer(...handlers)
server.listen({ onUnhandledRequest: 'error' })
server.listen({ onUnhandledRequest: 'warn' })
Copy link
Collaborator Author

@duyhungtnn duyhungtnn Nov 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In the new mswj version, setting onUnhandledRequest: 'error' throws an error for unhandled requests instead of doing nothing as before.

This change affects the test case for the ‘flush’ event.

To maintain the same test behavior as before, I switched it to just warn about unhandled requests.
@cre8ivejp what do you think ?

Copy link
Member

Choose a reason for hiding this comment

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

What other options do we have?
Can we rewrite the test or use another tool to avoid working around it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

here is the options

"warn" (Default) Print a warning but perform the request as-is.
"error" Print an error and halt request execution.
"bypass" Does not print anything and perform the request as-is.

======================

If we want to keep using the error option, We could update the flush test case @cre8ivejp

Copy link
Collaborator Author

@duyhungtnn duyhungtnn Nov 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I restored the code be3d641

The unhandled request was created because the cross-fetch was not working as expected.
We don't need to change any.

Copy link
Member

Choose a reason for hiding this comment

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

Nice catch!

return server
}

Expand Down
Loading