-
Notifications
You must be signed in to change notification settings - Fork 104
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
Further Enhancements to the Module System #1428
Changes from 7 commits
16ba5ad
c1a51dd
f8e3311
40b1eb3
d639512
6d81ad4
8acfd79
b00153e
a1f47f4
ef8d903
150e94e
509be3b
d565099
92d7b48
d493ffa
3ba38ef
813cceb
2a2ac1f
ba1cc1b
532a944
fbbb690
24ec58b
43481bb
9ed1e54
0380f5e
d67af01
bc6a53f
fb49b56
bd4b132
1e05b13
d1e5800
937bae9
d330c39
d06a2ca
f66472d
157cd3b
545f558
fd901b9
8553c76
f4fc2e2
2b7eb11
cdbac82
a3d9c4d
59dbf48
dfb5882
0dc35cc
5f8f68e
f264219
b05f072
a8f3951
657ab8c
9913230
288af5d
703da37
1d75fa2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ node_modules | |
dist/ | ||
.idea/ | ||
coverage/ | ||
yarn-error.log | ||
|
||
# emacs backup files | ||
*~ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
import { SourceMapConsumer } from "source-map" | ||
|
||
jest.mock('lodash', () => ({ | ||
...jest.requireActual('lodash'), | ||
memoize: jest.fn((x: any) => x), | ||
})) | ||
|
||
jest.mock('./src/modules/moduleLoaderAsync') | ||
jest.mock('./src/modules/moduleLoader') | ||
|
||
// @ts-ignore | ||
SourceMapConsumer.initialize({ | ||
'lib/mappings.wasm': 'https://unpkg.com/source-map@0.7.3/lib/mappings.wasm' | ||
}) | ||
|
||
global.fetch = jest.fn() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's better to mock only the functions/classes that each test suite needs within the test suite itself for the same reasons that we always try to reduce the scope of variables. If we were to declare all of our mocks here, you can imagine that it would very quickly become hard to keep track of which mocks are used in which test suite as new test cases are added/updated over time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that this is needed for the
source-map
library to work in browsers (aa58f3d). Is this really needed for the tests?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When running tests on my system, without this code in
jest.setup.ts
causesTypeError: Cannot read properties of undefined (reading 'then')
when running certain testsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I can't replicate this locally. Logically, this shouldn't be needed in a Node.js environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I just don't really know where the issue is, or how to resolve it at the moment