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

fix(c3): ensure shell scripts work on Windows #4445

Merged
merged 24 commits into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f30cd75
fix: ensure C3 shell scripts work on Windows
petebacondarwin Nov 15, 2023
02a3da6
C3 e2e - test on CI with Windows + pnpm
petebacondarwin Nov 15, 2023
ba452a4
C3 e2e - clean up test projects according to their type
petebacondarwin Nov 15, 2023
ed7fd0f
C3 - framework template fixes for Windows
petebacondarwin Nov 16, 2023
1cbc0f7
C3 e2e - make cleaning up e2e test files more resilient
petebacondarwin Nov 16, 2023
ed9761a
C3 - update Nuxt template to avoid requiring Windows incompatible she…
petebacondarwin Nov 16, 2023
92f02cc
C3 e2e - remove noise from expected test errors
petebacondarwin Nov 16, 2023
13c6c4e
C3 e2e - Only cleanup workers and projects that are over 1 hour old
petebacondarwin Nov 20, 2023
898b2e3
C3 e2e - only clean up test projects once per day at 3am
petebacondarwin Nov 20, 2023
e366929
C3 e2e - fix commit message tests on Windows
petebacondarwin Nov 16, 2023
4725c5c
C3 e2e - recreate test folders for each e2e retry
petebacondarwin Nov 16, 2023
adf2022
C3 e2e - bump test timeout longer
petebacondarwin Nov 16, 2023
1af8026
C3 e2e - do not run C3 e2e tests on unsupported OSes
petebacondarwin Nov 21, 2023
6e8dcb8
C3 e2e - store e2e logs separately by OS
petebacondarwin Nov 20, 2023
f77d164
C3 e2e - Retry some of the checks a few times to reduce flakiness
petebacondarwin Nov 20, 2023
a77cbed
C3 e2e - Record vitest results as a json artifact for downloading
petebacondarwin Nov 20, 2023
0dcb6cf
C3 e2e - disable global caches for package managers
petebacondarwin Nov 21, 2023
e658af7
C3 e2e - don't cancel other matrix jobs if one fails
petebacondarwin Nov 21, 2023
5d322d4
ci: retry the external-durable-objects-app fixture if it flakes
petebacondarwin Nov 21, 2023
880bc45
C3 e2e - drop docusaurus and react tests
petebacondarwin Nov 21, 2023
9d82d89
ci: skip fixture tests that exercise the "dev registry"
petebacondarwin Nov 21, 2023
799d3fa
C3 - ensure shell commands in logging are reasonably quoted
petebacondarwin Nov 23, 2023
aed7c14
fixup! C3 - ensure shell commands in logging are reasonably quoted
petebacondarwin Nov 24, 2023
c06c331
fixup! C3 - ensure shell commands in logging are reasonably quoted
petebacondarwin Nov 24, 2023
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
23 changes: 23 additions & 0 deletions .changeset/angry-walls-cheer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
"create-cloudflare": patch
---

fix: ensure shell scripts work on Windows

Our use of `shell-quote` was causing problems on Windows where it was
escaping character (such as `@`) by placing a backslash in front.
This made Windows think that such path arguments, were at the root.

For example, `npm install -D @cloudflare/workers-types` was being converted to
`npm install -D \@cloudflare/workers-types`, which resulted in errors like:

```
npm ERR! enoent ENOENT: no such file or directory, open 'D:\@cloudflare\workers-types\package.json'
```

Now we just rely directly on the Node.js `spawn` API to avoid any shell quoting
concerns. This has resulted in a slightly less streamlined experience for people
writing C3 plugins, but has the benefit that the developer doesn't have to worry
about quoting spawn arguments.

Closes https://github.com/cloudflare/workers-sdk/issues/4282
10 changes: 10 additions & 0 deletions .changeset/popular-dolls-enjoy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"create-cloudflare": patch
---

fix: update Nuxt template to work on Windows

Rather than relying upon the non-Windows shell syntax to specify an environment variable,
we now update the `nuxt.config.ts` files to include the cloudflare preset.

Fixes #4285
5 changes: 3 additions & 2 deletions .github/actions/run-c3-e2e/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ inputs:
runs:
using: "composite"
steps:
- name: Install Bun ${{ env.bun-version }}
- if: ${{ matrix.pm == 'bun' }}
name: Install Bun ${{ env.bun-version }}
uses: oven-sh/setup-bun@v1
with:
bun-version: ${{ env.bun-version }}
Expand All @@ -46,7 +47,7 @@ runs:
- name: Upload Logs
uses: actions/upload-artifact@v3
with:
name: e2e-logs
name: e2e-logs-${{matrix.os}}
path: packages/create-cloudflare/.e2e-logs

- name: Fail if errors detected
Expand Down
6 changes: 2 additions & 4 deletions .github/workflows/c3-e2e-project-cleanup.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@

name: C3 E2E Project Cleanup
on:
workflow_run:
workflows: [C3 E2E Tests, C3 E2E Tests (Dependabot), C3 E2E (Quarantine)]
types:
- completed
schedule:
- cron: "0 3 * * *" # Run at 3am each day
env:
node-version: 18.17.1
jobs:
Expand Down
9 changes: 7 additions & 2 deletions .github/workflows/c3-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,21 @@ env:
bun-version: 1.0.3
jobs:
e2e:
timeout-minutes: 30
timeout-minutes: 45
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-${{ matrix.os }}-${{ matrix.pm }}
cancel-in-progress: true
name: ${{ format('E2E ({0})', matrix.pm) }}
name: ${{ format('E2E ({0} on {1})', matrix.pm, matrix.os) }}
if: github.repository_owner == 'cloudflare' && github.event.pull_request.user.login != 'dependabot[bot]'
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest]
pm: [npm, pnpm, bun, yarn]
# include a single windows test with pnpm
include:
- os: windows-latest
pm: pnpm
runs-on: ${{ matrix.os }}
steps:
- name: Checkout Repo
Expand Down
2 changes: 1 addition & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"pgrep",
"PKCE",
"Positionals",
"qwik",
"reinitialise",
"scandir",
"selfsigned",
Expand All @@ -46,7 +47,6 @@
"websockets",
"workerd",
"xxhash",
"workerd",
"zjcompt"
],
"cSpell.ignoreWords": [
Expand Down
180 changes: 97 additions & 83 deletions fixtures/external-durable-objects-app/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,99 +5,113 @@ import { describe, expect, it, beforeAll, afterAll } from "vitest";
import type { ChildProcess } from "child_process";
import { type UnstableDevWorker, unstable_dev } from "wrangler";

describe.skip("Pages Functions", () => {
let a: UnstableDevWorker;
let b: UnstableDevWorker;
let c: UnstableDevWorker;
// TODO: reenable when https://github.com/cloudflare/workers-sdk/pull/4241 lands
// and improves reliability of this test.
describe.skip(
"Pages Functions",
() => {
let a: UnstableDevWorker;
let b: UnstableDevWorker;
let c: UnstableDevWorker;

let dWranglerProcess: ChildProcess;
let dIP: string;
let dPort: number;
let dResolveReadyPromise: (value: unknown) => void;
const dReadyPromise = new Promise((resolve) => {
dResolveReadyPromise = resolve;
});

beforeAll(async () => {
a = await unstable_dev(path.join(__dirname, "../a/index.ts"), {
config: path.join(__dirname, "../a/wrangler.toml"),
});
b = await unstable_dev(path.join(__dirname, "../b/index.ts"), {
config: path.join(__dirname, "../b/wrangler.toml"),
let dWranglerProcess: ChildProcess;
let dIP: string;
let dPort: number;
let dResolveReadyPromise: (value: unknown) => void;
const dReadyPromise = new Promise((resolve) => {
dResolveReadyPromise = resolve;
});

c = await unstable_dev(path.join(__dirname, "../c/index.ts"), {
config: path.join(__dirname, "../c/wrangler.toml"),
});
beforeAll(async () => {
a = await unstable_dev(path.join(__dirname, "../a/index.ts"), {
config: path.join(__dirname, "../a/wrangler.toml"),
});
b = await unstable_dev(path.join(__dirname, "../b/index.ts"), {
config: path.join(__dirname, "../b/wrangler.toml"),
});

dWranglerProcess = fork(
path.join("..", "..", "..", "packages", "wrangler", "bin", "wrangler.js"),
[
"pages",
"dev",
"public",
"--do=PAGES_REFERENCED_DO=MyDurableObject@a",
"--port=0",
"--inspector-port=0",
],
{
stdio: ["ignore", "ignore", "ignore", "ipc"],
cwd: path.resolve(__dirname, "..", "d"),
}
).on("message", (message) => {
const parsedMessage = JSON.parse(message.toString());
dIP = parsedMessage.ip;
dPort = parsedMessage.port;
dResolveReadyPromise(undefined);
c = await unstable_dev(path.join(__dirname, "../c/index.ts"), {
config: path.join(__dirname, "../c/wrangler.toml"),
});

dWranglerProcess = fork(
path.join(
"..",
"..",
"..",
"packages",
"wrangler",
"bin",
"wrangler.js"
),
[
"pages",
"dev",
"public",
"--do=PAGES_REFERENCED_DO=MyDurableObject@a",
"--port=0",
"--inspector-port=0",
],
{
stdio: ["ignore", "ignore", "ignore", "ipc"],
cwd: path.resolve(__dirname, "..", "d"),
}
).on("message", (message) => {
const parsedMessage = JSON.parse(message.toString());
dIP = parsedMessage.ip;
dPort = parsedMessage.port;
dResolveReadyPromise(undefined);
});
});
});

afterAll(async () => {
await dReadyPromise;
await a.stop();
await b.stop();
await c.stop();
afterAll(async () => {
await dReadyPromise;
await a.stop();
await b.stop();
await c.stop();

await new Promise((resolve, reject) => {
dWranglerProcess.once("exit", (code) => {
if (!code) {
resolve(code);
} else {
reject(code);
}
await new Promise((resolve, reject) => {
dWranglerProcess.once("exit", (code) => {
if (!code) {
resolve(code);
} else {
reject(code);
}
});
dWranglerProcess.kill("SIGTERM");
});
dWranglerProcess.kill("SIGTERM");
});
});

it("connects up Durable Objects and keeps state across wrangler instances", async () => {
await dReadyPromise;
it("connects up Durable Objects and keeps state across wrangler instances", async () => {
await dReadyPromise;

// Service registry is polled every 300ms,
// so let's give all the Workers a little time to find each other
await new Promise((resolve) => setTimeout(resolve, 700));
// Service registry is polled every 300ms,
// so let's give all the Workers a little time to find each other
await new Promise((resolve) => setTimeout(resolve, 700));

const responseA = await a.fetch(`/`, {
headers: {
"X-Reset-Count": "true",
},
const responseA = await a.fetch(`/`, {
headers: {
"X-Reset-Count": "true",
},
});
const dataA = (await responseA.json()) as { count: number; id: string };
expect(dataA.count).toEqual(1);
const responseB = await b.fetch(`/`);
const dataB = (await responseB.json()) as { count: number; id: string };
expect(dataB.count).toEqual(2);
const responseC = await c.fetch(`/`);
const dataC = (await responseC.json()) as { count: number; id: string };
expect(dataC.count).toEqual(3);
const responseD = await fetch(`http://${dIP}:${dPort}/`);
const dataD = (await responseD.json()) as { count: number; id: string };
expect(dataD.count).toEqual(4);
const responseA2 = await a.fetch(`/`);
const dataA2 = (await responseA2.json()) as { count: number; id: string };
expect(dataA2.count).toEqual(5);
expect(dataA.id).toEqual(dataB.id);
expect(dataA.id).toEqual(dataC.id);
expect(dataA.id).toEqual(dataA2.id);
});
const dataA = (await responseA.json()) as { count: number; id: string };
expect(dataA.count).toEqual(1);
const responseB = await b.fetch(`/`);
const dataB = (await responseB.json()) as { count: number; id: string };
expect(dataB.count).toEqual(2);
const responseC = await c.fetch(`/`);
const dataC = (await responseC.json()) as { count: number; id: string };
expect(dataC.count).toEqual(3);
const responseD = await fetch(`http://${dIP}:${dPort}/`);
const dataD = (await responseD.json()) as { count: number; id: string };
expect(dataD.count).toEqual(4);
const responseA2 = await a.fetch(`/`);
const dataA2 = (await responseA2.json()) as { count: number; id: string };
expect(dataA2.count).toEqual(5);
expect(dataA.id).toEqual(dataB.id);
expect(dataA.id).toEqual(dataC.id);
expect(dataA.id).toEqual(dataA2.id);
});
});
},
{ retry: 2 }
);
5 changes: 4 additions & 1 deletion fixtures/service-bindings-app/tests/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { describe, expect, it, beforeAll, afterAll } from "vitest";
import { UnstableDevWorker, unstable_dev } from "wrangler";
import path from "node:path";
describe("Service Bindings", () => {

// TODO: reenable when https://github.com/cloudflare/workers-sdk/pull/4241 lands
// and improves reliability of this test.
describe.skip("Service Bindings", () => {
let aWorker: UnstableDevWorker;

let bWorker: UnstableDevWorker;
Expand Down
Loading
Loading