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

Add tests for installWrangler #322

Merged
merged 2 commits into from
Nov 13, 2024
Merged

Add tests for installWrangler #322

merged 2 commits into from
Nov 13, 2024

Conversation

courtney-sims
Copy link
Contributor

@courtney-sims courtney-sims commented Nov 7, 2024

Baby step refactor to get more tests into this repo:

  • isolates index functions from side effects
  • creates type for config
  • uses config and package manager types for parameters to index functions so we can pass in some test data
  • adds some tests for the InstallWrangler method

Also pins the node version to avoid compatibility issues with the newly released version 23

Tested by running this version on a demo site:
Screenshot 2024-11-07 at 3 47 49 PM

@courtney-sims courtney-sims requested review from a team as code owners November 7, 2024 21:53
import { semverCompare } from "./utils";
import { getDetailedPagesDeployOutput } from "./wranglerArtifactManager";

export type WranglerActionConfig = z.infer<typeof wranglerActionConfig>;
Copy link
Member

@jrf0110 jrf0110 Nov 11, 2024

Choose a reason for hiding this comment

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

Now that we're defining a type for config, we should use it when instantiating config in index.ts here https://github.com/cloudflare/wrangler-action/pull/322/files#diff-a2a171449d862fe29692ce031981047d7ab755ae7f84c707aef80701b3ea0c80L40

/**
 * A configuration object that contains all the inputs & immutable state for the action.
 */
const config: WranglerActionConfig = {
	WRANGLER_VERSION: getInput("wranglerVersion") || DEFAULT_WRANGLER_VERSION,
	didUserProvideWranglerVersion: Boolean(getInput("wranglerVersion")),
	secrets: getMultilineInput("secrets"),
	workingDirectory: checkWorkingDirectory(getInput("workingDirectory")),
	CLOUDFLARE_API_TOKEN: getInput("apiToken"),
	CLOUDFLARE_ACCOUNT_ID: getInput("accountId"),
	ENVIRONMENT: getInput("environment"),
	VARS: getMultilineInput("vars"),
	COMMANDS: getMultilineInput("command"),
	QUIET_MODE: getBooleanInput("quiet"),
	PACKAGE_MANAGER: getInput("packageManager"),
	WRANGLER_OUTPUT_DIR: `${join(tmpdir(), "wranglerArtifacts")}`,
} ;

Or you could put the runtime validation there:

const config = WranglerActionConfig.parse({
	WRANGLER_VERSION: getInput("wranglerVersion") || DEFAULT_WRANGLER_VERSION,
	didUserProvideWranglerVersion: Boolean(getInput("wranglerVersion")),
	secrets: getMultilineInput("secrets"),
	workingDirectory: checkWorkingDirectory(getInput("workingDirectory")),
	CLOUDFLARE_API_TOKEN: getInput("apiToken"),
	CLOUDFLARE_ACCOUNT_ID: getInput("accountId"),
	ENVIRONMENT: getInput("environment"),
	VARS: getMultilineInput("vars"),
	COMMANDS: getMultilineInput("command"),
	QUIET_MODE: getBooleanInput("quiet"),
	PACKAGE_MANAGER: getInput("packageManager"),
	WRANGLER_OUTPUT_DIR: `${join(tmpdir(), "wranglerArtifacts")}`,
}  satisfies WranglerActionConfig);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Thanks!

@Maximo-Guk Maximo-Guk self-requested a review November 12, 2024 21:57
@@ -14,7 +14,7 @@ jobs:
- name: Setup Node.js
uses: actions/setup-node@v3
with:
node-version: "latest"
node-version: "22.11.0"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could just pin it to 22?

@courtney-sims courtney-sims merged commit cc4ede3 into main Nov 13, 2024
4 checks passed
@courtney-sims courtney-sims deleted the courtney-sims-testing branch November 13, 2024 16:34
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.

3 participants