Skip to content
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

Further wrangler init tests (and refactorings) #124

Merged
merged 7 commits into from
Dec 17, 2021

Conversation

petebacondarwin
Copy link
Contributor

See the individual commits...

  • Simplify log capturing in tests
  • test: align init error message with wrangler 1
  • test: always restore jest mocks after each test
  • test: move confirm() and prompt() into an object for easier mocking
  • fix: error and exit if init --type is used
  • test: ensure fixture clean up is resilient to missing files
  • fix: allow the user to exit init if there is already a toml file

@changeset-bot
Copy link

changeset-bot bot commented Dec 16, 2021

🦋 Changeset detected

Latest commit: 23e29e7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
wrangler Patch

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

@petebacondarwin petebacondarwin marked this pull request as ready for review December 16, 2021 21:42
Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

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

Love it, the output's so much cleaner too. Dropped some notes, but tentatively stamping anyway. Thanks for this PR!
image

packages/wrangler/package.json Show resolved Hide resolved
@@ -83,3 +83,9 @@ export async function prompt(text: string, type: "text" | "password" = "text") {
// ): Promise<{ label: string; value: string }>{

// }

// Export as an object so that it is easier to mock for testing
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be necessary, but I'll swing by next week and figure out mocking that'll let us keep the original exports. (Dropping this comment here to remind myself)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried a number of different ways to achieve this by "mocking" the module. But it either didn't work or was really messy. I would be interested to see your approach.

packages/wrangler/src/index.tsx Outdated Show resolved Hide resolved
packages/wrangler/src/index.tsx Outdated Show resolved Hide resolved
`compatibility_date = "${compatibilityDate}"` + "\n"
);
console.log(`✨ Succesfully created wrangler.toml`);
// TODO: suggest next steps?
Copy link
Contributor

Choose a reason for hiding this comment

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

In the future, this might be a great place to suggest a list of "starters" and automatically make a src with contents. This may be a good replacement for the generate command.

* then an error is thrown.
*/
function mockConfirm(...expectations: { text: string; result: boolean }[]) {
const mockImplementation = async (text: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

dope mock, solves the input problem neatly.

@petebacondarwin
Copy link
Contributor Author

petebacondarwin commented Dec 16, 2021

Thanks for the quick review (after hours!!)
I'll tidy this up and merge it tomorrow. Then you can fix the bits you don't like on Monday.

@petebacondarwin petebacondarwin merged commit 23543fe into cloudflare:main Dec 17, 2021
threepointone added a commit that referenced this pull request Dec 17, 2021
As mentioned in #124 (comment), it's possible to use regular exports from a module, but still be able to mock their implementations in jest. This PR does so, and passes tests.
petebacondarwin pushed a commit that referenced this pull request Dec 17, 2021
* export functions from dialogs.tsx, pass tests

As mentioned in #124 (comment), it's possible to use regular exports from a module, but still be able to mock their implementations in jest. This PR does so, and passes tests.
@github-actions github-actions bot mentioned this pull request Dec 17, 2021
@petebacondarwin petebacondarwin deleted the kv-namespace-tests-2 branch December 17, 2021 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants