-
Notifications
You must be signed in to change notification settings - Fork 785
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
fix(sys): make NodeLazyRequire complain if package versions aren't right #3346
Changes from all commits
64bac9f
da61eb2
382bb75
4f053a3
b1b8f20
cd403d4
5316818
8c71d85
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,68 @@ | ||||||||||||||||||||||||||||
import { NodeLazyRequire } from '../node-lazy-require'; | ||||||||||||||||||||||||||||
import { buildError } from '@utils'; | ||||||||||||||||||||||||||||
import { NodeResolveModule } from '../node-resolve-module'; | ||||||||||||||||||||||||||||
import fs from 'graceful-fs'; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const mockPackageJson = (version: string) => | ||||||||||||||||||||||||||||
JSON.stringify({ | ||||||||||||||||||||||||||||
version, | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
describe('node-lazy-require', () => { | ||||||||||||||||||||||||||||
describe('NodeLazyRequire', () => { | ||||||||||||||||||||||||||||
describe('ensure', () => { | ||||||||||||||||||||||||||||
let readFSMock: jest.SpyInstance<ReturnType<typeof fs.readFileSync>, Parameters<typeof fs.readFileSync>>; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
beforeEach(() => { | ||||||||||||||||||||||||||||
readFSMock = jest.spyOn(fs, 'readFileSync').mockReturnValue(mockPackageJson('10.10.10')); | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
afterEach(() => { | ||||||||||||||||||||||||||||
readFSMock.mockClear(); | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
function setup() { | ||||||||||||||||||||||||||||
const resolveModule = new NodeResolveModule(); | ||||||||||||||||||||||||||||
const nodeLazyRequire = new NodeLazyRequire(resolveModule, { | ||||||||||||||||||||||||||||
jest: ['2.0.7', '38.0.1'], | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
return nodeLazyRequire; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
it.each(['2.0.7', '10.10.10', '38.0.1', '38.0.2', '38.5.17'])( | ||||||||||||||||||||||||||||
'should not error if installed package has a suitable major version (%p)', | ||||||||||||||||||||||||||||
async (testVersion) => { | ||||||||||||||||||||||||||||
const nodeLazyRequire = setup(); | ||||||||||||||||||||||||||||
readFSMock.mockReturnValue(mockPackageJson(testVersion)); | ||||||||||||||||||||||||||||
let diagnostics = await nodeLazyRequire.ensure('.', ['jest']); | ||||||||||||||||||||||||||||
expect(diagnostics.length).toBe(0); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
it('should error if the installed version of a package is too low', async () => { | ||||||||||||||||||||||||||||
const nodeLazyRequire = setup(); | ||||||||||||||||||||||||||||
readFSMock.mockReturnValue(mockPackageJson('1.1.1')); | ||||||||||||||||||||||||||||
let [error] = await nodeLazyRequire.ensure('.', ['jest']); | ||||||||||||||||||||||||||||
expect(error).toEqual({ | ||||||||||||||||||||||||||||
...buildError([]), | ||||||||||||||||||||||||||||
header: 'Please install supported versions of dev dependencies with either npm or yarn.', | ||||||||||||||||||||||||||||
messageText: 'npm install --save-dev jest@38.0.1', | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
it.each(['100.1.1', '38.0.1-alpha.0'])( | ||||||||||||||||||||||||||||
'should error if the installed version of a package is too high (%p)', | ||||||||||||||||||||||||||||
async (version) => { | ||||||||||||||||||||||||||||
const nodeLazyRequire = setup(); | ||||||||||||||||||||||||||||
readFSMock.mockReturnValue(mockPackageJson(version)); | ||||||||||||||||||||||||||||
let [error] = await nodeLazyRequire.ensure('.', ['jest']); | ||||||||||||||||||||||||||||
expect(error).toEqual({ | ||||||||||||||||||||||||||||
...buildError([]), | ||||||||||||||||||||||||||||
header: 'Please install supported versions of dev dependencies with either npm or yarn.', | ||||||||||||||||||||||||||||
messageText: 'npm install --save-dev jest@38.0.1', | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
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. Could we add one more test here to verify that this returns an error for prereleases? E.g.
Suggested change
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. makes sense - I parameterized this test and added that version string, and then also added a few more to the other test which tests what should be accepted as valid versions — 3bd58e2 |
||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
}); |
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.
I removed this as a dependency since we were only using it in a few places and already depending on
semver
.I noticed references to this package in
scripts/license.ts
andNOTICE.md
— is there any manual intervention needed to sort those out?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.
possible nvm on this one - found
scripts/license.ts
and ran it, see 684fe50