-
Notifications
You must be signed in to change notification settings - Fork 240
fix(no-deprecated-functions): remove process.cwd
from resolve paths
#889
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
Merged
Merged
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
580ad23
fix(no-deprecated-functions): remove `process.cwd` from resolve paths
G-Rath a5e89de
refactor: move `detectJestVersion` into its own file
G-Rath 55adacc
chore: remove unneeded console log calls from test
G-Rath 1ca4d0a
chore: remove leftover comment
G-Rath b4ccd8b
fix(no-deprecated-functions): parse major `jest` version from string
G-Rath 26332f9
docs: expand on how jest version is determined and common workarounds
G-Rath c98efb4
docs: link to `no-deprecated-functions` from readme
G-Rath bf143f8
docs(no-deprecated-function): link to readme section about jest version
G-Rath File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,238 @@ | ||
import { spawnSync } from 'child_process'; | ||
import * as fs from 'fs'; | ||
import * as os from 'os'; | ||
import * as path from 'path'; | ||
import { JSONSchemaForNPMPackageJsonFiles } from '@schemastore/package'; | ||
import { create } from 'ts-node'; | ||
import { detectJestVersion } from '../detectJestVersion'; | ||
|
||
const compileFnCode = (pathToFn: string) => { | ||
const fnContents = fs.readFileSync(pathToFn, 'utf-8'); | ||
|
||
return create({ | ||
transpileOnly: true, | ||
compilerOptions: { sourceMap: false }, | ||
}).compile(fnContents, pathToFn); | ||
}; | ||
const compiledFn = compileFnCode(require.resolve('../detectJestVersion.ts')); | ||
const relativePathToFn = 'eslint-plugin-jest/lib/rules/detectJestVersion.js'; | ||
|
||
const runNodeScript = (cwd: string, script: string) => { | ||
return spawnSync('node', ['-e', script.split('\n').join(' ')], { | ||
cwd, | ||
encoding: 'utf-8', | ||
}); | ||
}; | ||
|
||
const runDetectJestVersion = (cwd: string) => { | ||
const out = runNodeScript( | ||
cwd, | ||
` | ||
try { | ||
console.log(require('${relativePathToFn}').detectJestVersion()); | ||
} catch (error) { | ||
console.error(error.message); | ||
} | ||
`, | ||
); | ||
|
||
console.log('status:', out.status); | ||
console.log('stdout:', out.stdout); | ||
console.log('stderr:', out.stderr); | ||
|
||
return out; | ||
}; | ||
|
||
/** | ||
* Makes a new temp directory, prefixed with `eslint-plugin-jest-` | ||
* | ||
* @return {Promise<string>} | ||
*/ | ||
const makeTempDir = () => | ||
fs.mkdtempSync(path.join(os.tmpdir(), 'eslint-plugin-jest-')); | ||
|
||
interface ProjectStructure { | ||
[key: `${string}/package.json`]: JSONSchemaForNPMPackageJsonFiles; | ||
[key: `${string}/${typeof relativePathToFn}`]: string; | ||
[key: `${string}/`]: null; | ||
'package.json'?: JSONSchemaForNPMPackageJsonFiles; | ||
} | ||
|
||
const setupFakeProject = (structure: ProjectStructure): string => { | ||
const tempDir = makeTempDir(); | ||
|
||
for (const [filePath, contents] of Object.entries(structure)) { | ||
if (contents === null) { | ||
fs.mkdirSync(path.join(tempDir, filePath), { recursive: true }); | ||
|
||
continue; | ||
} | ||
|
||
const folderPath = path.dirname(filePath); | ||
|
||
// make the directory (recursively) | ||
fs.mkdirSync(path.join(tempDir, folderPath), { recursive: true }); | ||
|
||
const finalContents = | ||
typeof contents === 'string' ? contents : JSON.stringify(contents); | ||
|
||
fs.writeFileSync(path.join(tempDir, filePath), finalContents); | ||
} | ||
|
||
return tempDir; | ||
}; | ||
|
||
// pin the original cwd so that we can restore it after each test | ||
// const projectDir = process.cwd(); | ||
|
||
// afterEach(() => process.chdir(projectDir)); | ||
|
||
describe('detectJestVersion', () => { | ||
describe('basic tests', () => { | ||
const packageJsonFactory = jest.fn<JSONSchemaForNPMPackageJsonFiles, []>(); | ||
|
||
beforeEach(() => { | ||
jest.resetModules(); | ||
jest.doMock(require.resolve('jest/package.json'), packageJsonFactory); | ||
}); | ||
|
||
describe('when the package.json is missing the version property', () => { | ||
it('throws an error', () => { | ||
packageJsonFactory.mockReturnValue({}); | ||
|
||
expect(() => detectJestVersion()).toThrow( | ||
/Unable to detect Jest version/iu, | ||
); | ||
}); | ||
}); | ||
|
||
it('caches versions', () => { | ||
packageJsonFactory.mockReturnValue({ version: '1.2.3' }); | ||
|
||
const version = detectJestVersion(); | ||
|
||
jest.resetModules(); | ||
|
||
expect(detectJestVersion).not.toThrow(); | ||
expect(detectJestVersion()).toBe(version); | ||
}); | ||
}); | ||
|
||
describe('when in a simple project', () => { | ||
it('finds the correct version', () => { | ||
const projectDir = setupFakeProject({ | ||
'package.json': { name: 'simple-project' }, | ||
[`node_modules/${relativePathToFn}` as const]: compiledFn, | ||
'node_modules/jest/package.json': { | ||
name: 'jest', | ||
version: '21.0.0', | ||
}, | ||
}); | ||
|
||
const { stdout, stderr } = runDetectJestVersion(projectDir); | ||
|
||
expect(stdout.trim()).toBe('21'); | ||
expect(stderr.trim()).toBe(''); | ||
}); | ||
}); | ||
|
||
describe('when in a hoisted mono-repo', () => { | ||
it('finds the correct version', () => { | ||
const projectDir = setupFakeProject({ | ||
'package.json': { name: 'mono-repo' }, | ||
[`node_modules/${relativePathToFn}` as const]: compiledFn, | ||
'node_modules/jest/package.json': { | ||
name: 'jest', | ||
version: '19.0.0', | ||
}, | ||
'packages/a/package.json': { name: 'package-a' }, | ||
'packages/b/package.json': { name: 'package-b' }, | ||
}); | ||
|
||
const { stdout, stderr } = runDetectJestVersion(projectDir); | ||
|
||
expect(stdout.trim()).toBe('19'); | ||
expect(stderr.trim()).toBe(''); | ||
}); | ||
}); | ||
|
||
describe('when in a subproject', () => { | ||
it('finds the correct versions', () => { | ||
const projectDir = setupFakeProject({ | ||
'backend/package.json': { name: 'package-a' }, | ||
[`backend/node_modules/${relativePathToFn}` as const]: compiledFn, | ||
'backend/node_modules/jest/package.json': { | ||
name: 'jest', | ||
version: '24.0.0', | ||
}, | ||
'frontend/package.json': { name: 'package-b' }, | ||
[`frontend/node_modules/${relativePathToFn}` as const]: compiledFn, | ||
'frontend/node_modules/jest/package.json': { | ||
name: 'jest', | ||
version: '15.0.0', | ||
}, | ||
}); | ||
|
||
const { stdout: stdoutBackend, stderr: stderrBackend } = | ||
runDetectJestVersion(path.join(projectDir, 'backend')); | ||
|
||
expect(stdoutBackend.trim()).toBe('24'); | ||
expect(stderrBackend.trim()).toBe(''); | ||
|
||
const { stdout: stdoutFrontend, stderr: stderrFrontend } = | ||
runDetectJestVersion(path.join(projectDir, 'frontend')); | ||
|
||
expect(stdoutFrontend.trim()).toBe('15'); | ||
expect(stderrFrontend.trim()).toBe(''); | ||
}); | ||
}); | ||
|
||
describe('when jest is not installed', () => { | ||
it('throws an error', () => { | ||
const projectDir = setupFakeProject({ | ||
'package.json': { name: 'no-jest' }, | ||
[`node_modules/${relativePathToFn}` as const]: compiledFn, | ||
'node_modules/pack/package.json': { name: 'pack' }, | ||
}); | ||
|
||
const { stdout, stderr } = runDetectJestVersion(projectDir); | ||
|
||
expect(stdout.trim()).toBe(''); | ||
expect(stderr.trim()).toContain('Unable to detect Jest version'); | ||
}); | ||
}); | ||
|
||
describe('when jest is changed on disk', () => { | ||
it('uses the cached version', () => { | ||
const projectDir = setupFakeProject({ | ||
'package.json': { name: 'no-jest' }, | ||
[`node_modules/${relativePathToFn}` as const]: compiledFn, | ||
'node_modules/jest/package.json': { name: 'jest', version: '26.0.0' }, | ||
}); | ||
|
||
const { stdout, stderr } = runNodeScript( | ||
projectDir, | ||
` | ||
const { detectJestVersion } = require('${relativePathToFn}'); | ||
const fs = require('fs'); | ||
|
||
console.log(detectJestVersion()); | ||
fs.writeFileSync( | ||
'node_modules/jest/package.json', | ||
JSON.stringify({ | ||
name: 'jest', | ||
version: '25.0.0', | ||
}), | ||
); | ||
console.log(detectJestVersion()); | ||
`, | ||
); | ||
|
||
const [firstCall, secondCall] = stdout.split('\n'); | ||
|
||
expect(firstCall).toBe('26'); | ||
expect(secondCall).toBe('26'); | ||
expect(stderr.trim()).toBe(''); | ||
}); | ||
}); | ||
}); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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.
hehe yeah this was the main "minor cleanup" I was going to do - I think we should be able to use changing the directory to remove the need to pass the
projectDir
around, which might be nicer.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.
So using
process.chdir
does work, but tbh I don't know which is actually nicer: using it means we don't have to pass around the project directory argument in all of our tests, but then we'll be magically changing directories and so stuff "just works" 🤔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.
process.chdir
actually changes the dir, so I'd prefer not to