Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

Fix migration test on Windows #221

Merged
merged 1 commit into from
Sep 7, 2018
Merged

Conversation

AWolf81
Copy link
Collaborator

@AWolf81 AWolf81 commented Sep 7, 2018

Related Issue / PR
#70

Summary:
During my work on codecov CI I noticed that migrations.test failed if I run yarn test on Windows.
Path.join returns a backslash on Windows. That's why I added a mock to it so it's returning the expected path.

@AWolf81 AWolf81 requested a review from kerm1it September 7, 2018 07:28
jest.mock('path', () => ({
join: () => 'test/guppy-projects',
}));

const ENGINE_KEY = 'test-key';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ENGINE_KEY seems unused as ESLint reported.
Should we remove it?

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, I believe I removed this in one of my PRs. Feel free to remove as well :)

Copy link
Collaborator

@kerm1it kerm1it left a comment

Choose a reason for hiding this comment

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

Good Catch! I only run tests on the Mac platform

Copy link
Owner

@joshwcomeau joshwcomeau left a comment

Choose a reason for hiding this comment

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

Thanks for fixing @AWolf81!

jest.mock('path', () => ({
join: () => 'test/guppy-projects',
}));

const ENGINE_KEY = 'test-key';
Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, I believe I removed this in one of my PRs. Feel free to remove as well :)

@joshwcomeau joshwcomeau merged commit 69ab961 into master Sep 7, 2018
@joshwcomeau joshwcomeau deleted the fix-migrations-test-on-windows branch September 7, 2018 11:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants