Skip to content

Fix --test-scheduled with custom builds & --x-dev-env #7164

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 5, 2024
Merged

Fix --test-scheduled with custom builds & --x-dev-env #7164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2a3edac
Fix test-scheduled
penalosa Nov 4, 2024
4f3d87c
Create sweet-toes-check.md
penalosa Nov 4, 2024
3306824
fix tests
penalosa Nov 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/sweet-toes-check.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

Fix `--test-scheduled` with custom builds & `--x-dev-env`
24 changes: 24 additions & 0 deletions packages/wrangler/e2e/__snapshots__/dev.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,47 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`basic js dev: 'wrangler dev --no-x-dev-env' > --test-scheduled works with wrangler dev --no-x-dev-env > custom build 1`] = `"Ran scheduled event"`;

exports[`basic js dev: 'wrangler dev --no-x-dev-env' > --test-scheduled works with wrangler dev --no-x-dev-env > no custom build 1`] = `"Ran scheduled event"`;

exports[`basic js dev: 'wrangler dev --no-x-dev-env' > --test-scheduled works with wrangler dev --no-x-dev-env 1`] = `"Ran scheduled event"`;

exports[`basic js dev: 'wrangler dev --no-x-dev-env' > can modify worker during wrangler dev --no-x-dev-env 1`] = `"Hello World!"`;

exports[`basic js dev: 'wrangler dev --no-x-dev-env' > can modify worker during wrangler dev --no-x-dev-env 2`] = `"Updated Worker! value"`;

exports[`basic js dev: 'wrangler dev --no-x-dev-env' > hotkeys can be disabled with wrangler dev --no-x-dev-env 1`] = `"Hello World!"`;

exports[`basic js dev: 'wrangler dev --remote --no-x-dev-env' > --test-scheduled works with wrangler dev --remote --no-x-dev-env > custom build 1`] = `"Ran scheduled event"`;

exports[`basic js dev: 'wrangler dev --remote --no-x-dev-env' > --test-scheduled works with wrangler dev --remote --no-x-dev-env > no custom build 1`] = `"Ran scheduled event"`;

exports[`basic js dev: 'wrangler dev --remote --no-x-dev-env' > --test-scheduled works with wrangler dev --remote --no-x-dev-env 1`] = `"Ran scheduled event"`;

exports[`basic js dev: 'wrangler dev --remote --no-x-dev-env' > can modify worker during wrangler dev --remote --no-x-dev-env 1`] = `"Hello World!"`;

exports[`basic js dev: 'wrangler dev --remote --no-x-dev-env' > can modify worker during wrangler dev --remote --no-x-dev-env 2`] = `"Updated Worker! value"`;

exports[`basic js dev: 'wrangler dev --remote --no-x-dev-env' > hotkeys can be disabled with wrangler dev --remote --no-x-dev-env 1`] = `"Hello World!"`;

exports[`basic js dev: 'wrangler dev --remote --x-dev-env' > --test-scheduled works with wrangler dev --remote --x-dev-env > custom build 1`] = `"Ran scheduled event"`;

exports[`basic js dev: 'wrangler dev --remote --x-dev-env' > --test-scheduled works with wrangler dev --remote --x-dev-env > no custom build 1`] = `"Ran scheduled event"`;

exports[`basic js dev: 'wrangler dev --remote --x-dev-env' > --test-scheduled works with wrangler dev --remote --x-dev-env 1`] = `"Ran scheduled event"`;

exports[`basic js dev: 'wrangler dev --remote --x-dev-env' > can modify worker during wrangler dev --remote --x-dev-env 1`] = `"Hello World!"`;

exports[`basic js dev: 'wrangler dev --remote --x-dev-env' > can modify worker during wrangler dev --remote --x-dev-env 2`] = `"Updated Worker! value"`;

exports[`basic js dev: 'wrangler dev --remote --x-dev-env' > hotkeys can be disabled with wrangler dev --remote --x-dev-env 1`] = `"Hello World!"`;

exports[`basic js dev: 'wrangler dev --x-dev-env' > --test-scheduled works with wrangler dev --x-dev-env > custom build 1`] = `"Ran scheduled event"`;

exports[`basic js dev: 'wrangler dev --x-dev-env' > --test-scheduled works with wrangler dev --x-dev-env > no custom build 1`] = `"Ran scheduled event"`;

exports[`basic js dev: 'wrangler dev --x-dev-env' > --test-scheduled works with wrangler dev --x-dev-env 1`] = `"Ran scheduled event"`;

exports[`basic js dev: 'wrangler dev --x-dev-env' > can modify worker during wrangler dev --x-dev-env 1`] = `"Hello World!"`;

exports[`basic js dev: 'wrangler dev --x-dev-env' > can modify worker during wrangler dev --x-dev-env 2`] = `"Updated Worker! value"`;
Expand Down
70 changes: 70 additions & 0 deletions packages/wrangler/e2e/dev.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,76 @@ describe.each([

await expect(worker.currentOutput).not.toContain("[b] open a browser");
});

describe(`--test-scheduled works with ${cmd}`, async () => {
it("custom build", async () => {
const helper = new WranglerE2ETestHelper();
await helper.seed({
"wrangler.toml": dedent`
name = "${workerName}"
main = "src/index.ts"
compatibility_date = "2023-01-01"
[build]
command = "true"
`,
"src/index.ts": dedent`
export default {
scheduled(event) {
console.log("Event triggered")
}
}`,
"package.json": dedent`
{
"name": "worker",
"version": "0.0.0",
"private": true
}
`,
});
const worker = helper.runLongLived(`${cmd} --test-scheduled`);

const { url } = await worker.waitForReady();

await expect(
fetch(`${url}/__scheduled`).then((r) => r.text())
).resolves.toMatchSnapshot();

await worker.readUntil(/Event triggered/);
});

it("no custom build", async () => {
const helper = new WranglerE2ETestHelper();
await helper.seed({
"wrangler.toml": dedent`
name = "${workerName}"
main = "src/index.ts"
compatibility_date = "2023-01-01"
`,
"src/index.ts": dedent`
export default {
scheduled(event) {
console.log("Event triggered")
}
}`,
"package.json": dedent`
{
"name": "worker",
"version": "0.0.0",
"private": true
}
`,
});
const worker = helper.runLongLived(`${cmd} --test-scheduled`);

const { url } = await worker.waitForReady();

await expect(
fetch(`${url}/__scheduled`).then((r) => r.text())
).resolves.toMatchSnapshot();

await worker.readUntil(/Event triggered/);
});
});
});

// Skipping remote python tests because they consistently flake with timeouts
Expand Down
2 changes: 2 additions & 0 deletions packages/wrangler/src/__tests__/navigator-user-agent.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ describe("defineNavigatorUserAgent is respected", () => {
exports: [],
},
path.resolve("dist"),
// @ts-expect-error Ignore the requirement for passing undefined values
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Just wanted to avoid @ts-expect-error. This might also make the test clearer which options the tests really cares.

{
  // Extracted from the original options
  ...defaultBundleOptions,
  defineNavigatorUserAgent: false,
}

{
bundle: true,
additionalModules: [],
Expand Down Expand Up @@ -172,6 +173,7 @@ describe("defineNavigatorUserAgent is respected", () => {
exports: [],
},
path.resolve("dist"),
// @ts-expect-error Ignore the requirement for passing undefined values
{
bundle: true,
additionalModules: [],
Expand Down
14 changes: 14 additions & 0 deletions packages/wrangler/src/api/startDevWorker/BundlerController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,20 @@ export class BundlerController extends Controller<BundlerControllerEventMap> {
config.compatibilityDate,
config.compatibilityFlags
),
testScheduled: config.dev.testScheduled,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the actual fix for the reported issue

plugins: undefined,

// Pages specific options used by wrangler pages commands
entryName: undefined,
inject: undefined,
isOutfile: undefined,
external: undefined,

// We don't use esbuild watching for custom builds
watch: undefined,

// sourcemap defaults to true in dev
sourcemap: undefined,
});
if (buildAborter.signal.aborted) {
return;
Expand Down
1 change: 1 addition & 0 deletions packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ async function convertToConfigBundle(
services: bindings.services,
serviceBindings: fetchers,
bindVectorizeToProd: event.config.dev?.bindVectorizeToProd ?? false,
testScheduled: !!event.config.dev.testScheduled,
};
}

Expand Down
10 changes: 10 additions & 0 deletions packages/wrangler/src/deploy/deploy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,16 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
props.compatibilityFlags ?? config.compatibility_flags
),
plugins: [logBuildOutput(nodejsCompatMode)],

// Pages specific options used by wrangler pages commands
entryName: undefined,
inject: undefined,
isOutfile: undefined,
external: undefined,

// These options are dev-only
testScheduled: undefined,
watch: undefined,
}
);

Expand Down
37 changes: 16 additions & 21 deletions packages/wrangler/src/deployment-bundle/bundle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,32 +115,31 @@ export type BundleOptions = {
// A module collector enables you to observe what modules are in the Worker.
moduleCollector: ModuleCollector;
serveLegacyAssetsFromWorker: boolean;
legacyAssets?: Config["legacy_assets"];
bypassAssetCache?: boolean;
legacyAssets: Config["legacy_assets"] | undefined;
bypassAssetCache: boolean | undefined;
doBindings: DurableObjectBindings;
workflowBindings: WorkflowBinding[];
jsxFactory?: string;
jsxFragment?: string;
entryName?: string;
watch?: boolean;
tsconfig?: string;
minify?: boolean;
nodejsCompatMode?: NodeJSCompatMode;
jsxFactory: string | undefined;
jsxFragment: string | undefined;
entryName: string | undefined;
watch: boolean | undefined;
tsconfig: string | undefined;
minify: boolean | undefined;
nodejsCompatMode: NodeJSCompatMode | undefined;
define: Config["define"];
alias: Config["alias"];
checkFetch: boolean;
mockAnalyticsEngineDatasets: Config["analytics_engine_datasets"];
targetConsumer: "dev" | "deploy";
testScheduled?: boolean;
inject?: string[];
loader?: Record<string, string>;
sourcemap?: esbuild.CommonOptions["sourcemap"];
plugins?: esbuild.Plugin[];
isOutfile?: boolean;
testScheduled: boolean | undefined;
inject: string[] | undefined;
sourcemap: esbuild.CommonOptions["sourcemap"] | undefined;
plugins: esbuild.Plugin[] | undefined;
isOutfile: boolean | undefined;
local: boolean;
projectRoot: string | undefined;
defineNavigatorUserAgent: boolean;
external?: string[];
external: string[] | undefined;
};

/**
Expand Down Expand Up @@ -172,7 +171,6 @@ export async function bundleWorker(
targetConsumer,
testScheduled,
inject: injectOption,
loader,
sourcemap,
plugins,
isOutfile,
Expand Down Expand Up @@ -424,10 +422,7 @@ export async function bundleWorker(
...define,
},
}),
loader: {
...COMMON_ESBUILD_OPTIONS.loader,
...(loader || {}),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The loader option wasn't used anywhere, so I removed it

},
loader: COMMON_ESBUILD_OPTIONS.loader,
plugins: [
aliasPlugin,
moduleCollector.plugin,
Expand Down
1 change: 1 addition & 0 deletions packages/wrangler/src/dev/local.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ export async function localPropsToConfigBundle(
services: props.services,
serviceBindings,
bindVectorizeToProd: props.bindVectorizeToProd,
testScheduled: !!props.testScheduled,
};
}

Expand Down
3 changes: 2 additions & 1 deletion packages/wrangler/src/dev/miniflare.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ export interface ConfigBundle {
services: Config["services"] | undefined;
serviceBindings: Record<string, ServiceFetch>;
bindVectorizeToProd: boolean;
testScheduled: boolean;
}

export class WranglerLog extends Log {
Expand Down Expand Up @@ -908,7 +909,7 @@ export async function buildMiniflareOptions(
internalObjects: CfDurableObject[];
entrypointNames: string[];
}> {
if (config.crons.length > 0) {
if (config.crons.length > 0 && !config.testScheduled) {
if (!didWarnMiniflareCronSupport) {
didWarnMiniflareCronSupport = true;
log.warn(
Expand Down
10 changes: 10 additions & 0 deletions packages/wrangler/src/dev/start-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,16 @@ async function runEsbuild({
workflowBindings,
projectRoot,
defineNavigatorUserAgent,

// Pages specific options used by wrangler pages commands
entryName: undefined,
inject: undefined,
isOutfile: undefined,
external: undefined,

watch: undefined,
sourcemap: undefined,
plugins: undefined,
})
: undefined;

Expand Down
9 changes: 9 additions & 0 deletions packages/wrangler/src/dev/use-esbuild.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,15 @@ export function runBuild(
local,
projectRoot,
defineNavigatorUserAgent,

// Pages specific options used by wrangler pages commands
entryName: undefined,
inject: undefined,
isOutfile: undefined,
external: undefined,

// sourcemap defaults to true in dev
sourcemap: undefined,
})
: undefined;

Expand Down
8 changes: 8 additions & 0 deletions packages/wrangler/src/pages/functions/buildPlugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,5 +114,13 @@ export function buildPluginFromFunctions({
local,
projectRoot: getPagesProjectRoot(),
defineNavigatorUserAgent,

legacyAssets: undefined,
bypassAssetCache: undefined,
jsxFactory: undefined,
jsxFragment: undefined,
tsconfig: undefined,
testScheduled: undefined,
isOutfile: undefined,
});
}
18 changes: 17 additions & 1 deletion packages/wrangler/src/pages/functions/buildWorker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export function buildWorkerFromFunctions({
additionalModules: [],
moduleCollector,
inject: [routesModule],
...(outdir ? { entryName: "index" } : {}),
...(outdir ? { entryName: "index" } : { entryName: undefined }),
minify,
sourcemap,
watch,
Expand All @@ -90,6 +90,13 @@ export function buildWorkerFromFunctions({
local,
projectRoot: getPagesProjectRoot(),
defineNavigatorUserAgent,

legacyAssets: undefined,
bypassAssetCache: undefined,
jsxFactory: undefined,
jsxFragment: undefined,
tsconfig: undefined,
testScheduled: undefined,
});
}

Expand Down Expand Up @@ -195,6 +202,15 @@ export function buildRawWorker({
local,
projectRoot: getPagesProjectRoot(),
defineNavigatorUserAgent,

legacyAssets: undefined,
bypassAssetCache: undefined,
jsxFactory: undefined,
jsxFragment: undefined,
tsconfig: undefined,
testScheduled: undefined,
entryName: undefined,
inject: undefined,
});
}

Expand Down
12 changes: 12 additions & 0 deletions packages/wrangler/src/versions/upload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from "../deployment-bundle/bundle-reporter";
import { getBundleType } from "../deployment-bundle/bundle-type";
import { createWorkerUploadForm } from "../deployment-bundle/create-worker-upload-form";
import { logBuildOutput } from "../deployment-bundle/esbuild-plugins/log-build-output";
import {
findAdditionalModules,
writeAdditionalModules,
Expand Down Expand Up @@ -321,6 +322,17 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
props.compatibilityDate ?? config.compatibility_date,
props.compatibilityFlags ?? config.compatibility_flags
),
plugins: [logBuildOutput(nodejsCompatMode)],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a separate fix to unify versions & deploy messaging


// Pages specific options used by wrangler pages commands
entryName: undefined,
inject: undefined,
isOutfile: undefined,
external: undefined,

// These options are dev-only
testScheduled: undefined,
watch: undefined,
}
);

Expand Down
Loading