-
Notifications
You must be signed in to change notification settings - Fork 0
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
chore(testing): add integration test for init command #29
chore(testing): add integration test for init command #29
Conversation
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.
Great work 🔥 🔥 just a few questions
afterEach(() => { | ||
jest.resetModules(); | ||
}); | ||
describe("my thing", () => { |
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.
should probably use something more descriptive 😂
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.
🤦♂️ that's embarrassing
jest.mock("fs"); | ||
jest.spyOn(process, "cwd").mockImplementation(() => "/custom/path/to"); | ||
|
||
const { getTemplateLocation } = require("../utils/getTemplateLocation"); |
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.
Why's this getting reimported?
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.
Yes! Great question. I asked the same thing (Alvin wrote the original test).
The reason for that is this: we're spying on process.cwd()
, which is used inside the getTemplateLocation
function's file. By reimporting it, within the it
block after we've mocked the implementation for process.cwd
, the mock works as expected and the test assertion passes.
If we remove that and use the "global" import for that file, process.cwd()
does not use our mock. I found this out through trial and error, and from reading these relevant Jest issues:
I will add a comment to explain that (condensed of course). Great question!
src/__tests__/init.test.ts
Outdated
} | ||
}); | ||
|
||
it("expects to find a backup.json and package.json", async () => { |
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.
Should we be checking for yarn.lock
to exist here as well?
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.
Jeffery, your eyes are impeccable - great catch - I'll add that in now
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.
@all-contributors please add @jeffreyzhen for review |
I've put up a pull request to add @jeffreyzhen! 🎉 |
This PR adds some integration tests for the
init
commandChanges
Adds tests for the following:
init
command works without a flaginit
command works with a flag of a valid projectinit
command displays an error when you pass an invalid flaginit
command installseb-scripts
and adds a script to thepackage.json
Screenshots
Checklist
Fixes #19