-
Notifications
You must be signed in to change notification settings - Fork 67
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 E2E tests #117
Fix E2E tests #117
Conversation
🦋 Changeset detectedLatest commit: 239d2a7 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
async function readCensoredPackageJson( | ||
packageJsonPath: string, | ||
): Promise<unknown> { | ||
/* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return */ | ||
function censorVersions(packageJson: any): any { | ||
function censorDiffering(packageJson: any): any { | ||
packageJson.author = '?'; |
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.
If you ran locally this would contain your machine's commit author which also broke snapshots.
// @ts-ignore | ||
env: { | ||
CI: 'true', | ||
}, |
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.
This turned out to be one of the reasons it was hanging when I ran the tests locally. I guess we were in watch mode...
@@ -498,6 +504,12 @@ describe('creating a new project', () => { | |||
const output = await execa('yarnpkg', ['test'], { | |||
cwd: repoDirectory, | |||
all: true, | |||
reject: false, |
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.
We never want to treat a non-zero exit code as an error, since this would mean not testing the snapshot.
'.git', | ||
'.DS_Store', | ||
const defaultIgnores = ['node_modules', '.git', '.DS_Store']; | ||
const defaultHashIgnores = [ | ||
// adding lockfiles here because it can be different | ||
// on different runs; since we install the latest versions | ||
// of some packages when making a repository | ||
'yarn.lock', | ||
'package-lock.json', |
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 amended @threepointone's original code to not completely hide the lock files but instead to make it so that hashes are not generated for them.
This allowed me to notice that we were creating package-lock.json
files erroneously.
I think this is the equivalent of the replacement of variances with ?
that I did in my earlier PR.
execSync('git', ['init'], { | ||
cwd: newModularRoot, | ||
}); | ||
} |
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.
Within OSX deleting files from a temp directory sometimes produced EINVAL
errors. This is apparently a common problem and was causing the tests to fail on my local machine on this line.
We side-step the issue by initialising a Git repository ourselves, since this causes CRA to not create a gigantic .git
directory that we then need to delete. I presume there might be a fix to fs-extras
that should be made, but that seems like more work...
fs.removeSync(path.join(appPath, 'yarn.lock')); | ||
fs.removeSync(path.join(appPath, 'package-lock.json')); |
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.
This file is being created due to how react-scripts
detects a Yarn installation not being appropriate within a monorepo.
The real fix would probably be to PR into their project a Yarn detection which used find-up
to look for a yarn.lock
file in the parent, however doing so would raise eyebrows unless we were also fixing #1333.
112a128
to
aea2539
Compare
aea2539
to
3932099
Compare
Remove `package-lock.json` file created by adding a new app. Initialise a Git repository before `create-react-app` does. Fixes #113
3932099
to
239d2a7
Compare
Fixes #113.