Skip to content

Commit 44b9e8f

Browse files
authored
refactor: Claude refactoring (#757)
* feat: setting terminal * feat: claude hooks * feat: skill workers-best-practices * refactor: concurrent calls * chore: build * feat: skill wrangler * refactor: switch from exec to execFile * refactor: setTimeout NODE_ENV check for tests * fix: build failure now terminate polling * Revert "feat: skill wrangler" This reverts commit a1de25a. * Revert "feat: skill workers-best-practices" This reverts commit fee66d7.
1 parent 37cc643 commit 44b9e8f

19 files changed

Lines changed: 321 additions & 81 deletions

File tree

.changeset/brown-months-accept.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"github-actions-cloudflare-pages": patch
3+
---
4+
5+
fix: build failure now terminate polling

.changeset/grumpy-files-dance.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"github-actions-cloudflare-pages": patch
3+
---
4+
5+
refactor: setTimeout NODE_ENV check for tests

.changeset/wicked-meals-wave.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"github-actions-cloudflare-pages": patch
3+
---
4+
5+
refactor: switch from exec to execFile. Stops risk of spaces or special characters breaking the command.

.claude/settings.json

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
{
2+
"hooks": {
3+
"Stop": [
4+
{
5+
"hooks": [
6+
{
7+
"type": "command",
8+
"command": "bash ./.github/hooks/scripts/stop-type-check.sh",
9+
"timeout": 60
10+
}
11+
]
12+
},
13+
{
14+
"hooks": [
15+
{
16+
"type": "command",
17+
"command": "bash ./.github/hooks/scripts/stop-review-agents.sh",
18+
"timeout": 10
19+
}
20+
]
21+
}
22+
],
23+
"PostToolUse": [
24+
{
25+
"hooks": [
26+
{
27+
"type": "command",
28+
"command": "if [[ -n \"$TOOL_INPUT_FILE_PATH\" && -f \"$TOOL_INPUT_FILE_PATH\" ]]; then bash ./.github/hooks/scripts/pre-commit-oxc.sh \"$TOOL_INPUT_FILE_PATH\"; fi"
29+
}
30+
]
31+
}
32+
]
33+
}
34+
}

.vscode/settings.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,6 @@
2323
},
2424
"editor.suggest.showKeywords": false,
2525
"oxc.enable.oxfmt": true,
26-
"oxc.enable.oxlint": true
26+
"oxc.enable.oxlint": true,
27+
"terminal.integrated.defaultProfile.linux": "zsh"
2728
}

__generated__/responses/api.cloudflare.com/pages/deployments/deployments.idle.response.json

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

__tests__/common/cloudflare/deployment/create.test.ts

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
CLOUDFLARE_API_TOKEN,
99
createCloudflareDeployment
1010
} from '@/common/cloudflare/deployment/create.js'
11-
import {execAsync} from '@/common/utils.js'
11+
import {execFileAsync} from '@/common/utils.js'
1212
import {INPUT_KEY_WORKING_DIRECTORY} from '@/input-keys'
1313
import RESPONSE_NOT_FOUND_DEPLOYMENTS from '@/responses/api.cloudflare.com/pages/deployments/deployments-not-found.response.json' with {type: 'json'}
1414
import RESPONSE_DEPLOYMENTS_IDLE from '@/responses/api.cloudflare.com/pages/deployments/deployments.idle.response.json' with {type: 'json'}
@@ -35,15 +35,18 @@ describe(createCloudflareDeployment, () => {
3535
afterEach(async () => {
3636
mockApi.mockAgent.assertNoPendingInterceptors()
3737
await mockApi.mockAgent.close()
38-
vi.mocked(execAsync).mockReset()
38+
vi.mocked(execFileAsync).mockReset()
3939
vi.runOnlyPendingTimers()
4040
vi.useRealTimers()
4141
})
4242

4343
test('handles thrown error from wrangler deploy', async () => {
4444
expect.assertions(10)
4545

46-
vi.mocked(execAsync).mockRejectedValueOnce({stderr: 'Oh no!', stdout: ''})
46+
vi.mocked(execFileAsync).mockRejectedValueOnce({
47+
stderr: 'Oh no!',
48+
stdout: ''
49+
})
4750

4851
// Expect Cloudflare Api Token and Account Id to be undefined.
4952
expect(process.env[CLOUDFLARE_API_TOKEN]).toBeUndefined()
@@ -57,8 +60,21 @@ describe(createCloudflareDeployment, () => {
5760
})
5861
).rejects.toThrowErrorMatchingInlineSnapshot(`[Error: Oh no!]`)
5962

60-
expect(execAsync).toHaveBeenCalledWith(
61-
`npx wrangler@${packageJson.devDependencies.wrangler} pages deploy mock-directory --project-name=mock-cloudflare-project-name --branch=mock-github-head-ref --commit-dirty=true --commit-hash=mock-github-sha`,
63+
expect(execFileAsync).toHaveBeenCalledWith(
64+
'npx',
65+
[
66+
`wrangler@${packageJson.devDependencies.wrangler}`,
67+
'pages',
68+
'deploy',
69+
'mock-directory',
70+
'--project-name',
71+
'mock-cloudflare-project-name',
72+
'--branch',
73+
'mock-github-head-ref',
74+
'--commit-dirty=true',
75+
'--commit-hash',
76+
'mock-github-sha'
77+
],
6278
{
6379
// oxlint-disable-next-line typescript/no-unsafe-assignment
6480
env: expect.objectContaining({
@@ -69,7 +85,7 @@ describe(createCloudflareDeployment, () => {
6985
}
7086
)
7187

72-
expect(execAsync).toHaveBeenCalledTimes(1)
88+
expect(execFileAsync).toHaveBeenCalledTimes(1)
7389
expect(info).not.toHaveBeenCalled()
7490
expect(process.env[CLOUDFLARE_API_TOKEN]).toBe(
7591
'mock-cloudflare-api-token'
@@ -85,7 +101,7 @@ describe(createCloudflareDeployment, () => {
85101
test('handles thrown error from getDeployments', async () => {
86102
expect.assertions(5)
87103

88-
vi.mocked(execAsync).mockResolvedValueOnce({
104+
vi.mocked(execFileAsync).mockResolvedValueOnce({
89105
stdout: 'success',
90106
stderr: ''
91107
})
@@ -105,7 +121,7 @@ describe(createCloudflareDeployment, () => {
105121
).rejects.toThrowErrorMatchingInlineSnapshot(
106122
`[ParseError: A request to the Cloudflare API (https://api.cloudflare.com/client/v4/accounts/mock-cloudflare-account-id/pages/projects/mock-cloudflare-project-name/deployments) failed.]`
107123
)
108-
expect(execAsync).toHaveBeenCalledTimes(1)
124+
expect(execFileAsync).toHaveBeenCalledTimes(1)
109125
expect(info).toHaveBeenLastCalledWith('success')
110126
expect(setOutput).not.toHaveBeenCalled()
111127
expect(summary.addTable).not.toHaveBeenCalled()
@@ -116,7 +132,7 @@ describe(createCloudflareDeployment, () => {
116132

117133
stubInputEnv(INPUT_KEY_WORKING_DIRECTORY)
118134

119-
vi.mocked(execAsync).mockResolvedValueOnce({
135+
vi.mocked(execFileAsync).mockResolvedValueOnce({
120136
stdout: 'success',
121137
stderr: ''
122138
})
@@ -143,8 +159,21 @@ describe(createCloudflareDeployment, () => {
143159
})
144160
// vi.advanceTimersByTime(2000)
145161

146-
expect(execAsync).toHaveBeenCalledWith(
147-
`npx wrangler@${packageJson.devDependencies.wrangler} pages deploy mock-directory --project-name=mock-cloudflare-project-name --branch=mock-github-head-ref --commit-dirty=true --commit-hash=mock-github-sha`,
162+
expect(execFileAsync).toHaveBeenCalledWith(
163+
'npx',
164+
[
165+
`wrangler@${packageJson.devDependencies.wrangler}`,
166+
'pages',
167+
'deploy',
168+
'mock-directory',
169+
'--project-name',
170+
'mock-cloudflare-project-name',
171+
'--branch',
172+
'mock-github-head-ref',
173+
'--commit-dirty=true',
174+
'--commit-hash',
175+
'mock-github-sha'
176+
],
148177
{
149178
// oxlint-disable-next-line typescript/no-unsafe-assignment
150179
env: expect.objectContaining({
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest'
2+
3+
import type {PagesDeployment} from '@/common/cloudflare/types.js'
4+
import type {MockApi} from '@/tests/helpers/api.js'
5+
6+
import {statusCloudflareDeployment} from '@/common/cloudflare/deployment/status.js'
7+
import {sleep} from '@/common/utils.js'
8+
import RESPONSE_DEPLOYMENTS_IDLE from '@/responses/api.cloudflare.com/pages/deployments/deployments.idle.response.json' with {type: 'json'}
9+
import RESPONSE_DEPLOYMENTS from '@/responses/api.cloudflare.com/pages/deployments/deployments.response.json' with {type: 'json'}
10+
import {
11+
MOCK_ACCOUNT_ID,
12+
MOCK_API_PATH_DEPLOYMENTS,
13+
MOCK_PROJECT_NAME,
14+
setMockApi
15+
} from '@/tests/helpers/api.js'
16+
17+
vi.mock(import('@/common/utils.js'))
18+
vi.mock(import('@actions/core'))
19+
20+
const API_ENDPOINT = {
21+
accountId: MOCK_ACCOUNT_ID,
22+
projectName: MOCK_PROJECT_NAME
23+
}
24+
25+
type LatestStage = PagesDeployment['latest_stage']
26+
27+
const withLatestStage = (
28+
name: LatestStage['name'],
29+
status: LatestStage['status']
30+
): typeof RESPONSE_DEPLOYMENTS => ({
31+
...RESPONSE_DEPLOYMENTS,
32+
result: RESPONSE_DEPLOYMENTS.result.map((deployment, index) =>
33+
index === 0
34+
? {
35+
...deployment,
36+
latest_stage: {
37+
name,
38+
status,
39+
started_on: null,
40+
ended_on: null
41+
} as LatestStage
42+
}
43+
: deployment
44+
) as (typeof RESPONSE_DEPLOYMENTS)['result']
45+
})
46+
47+
describe(statusCloudflareDeployment, () => {
48+
let mockApi: MockApi
49+
50+
beforeEach(() => {
51+
mockApi = setMockApi()
52+
})
53+
54+
afterEach(async () => {
55+
mockApi.mockAgent.assertNoPendingInterceptors()
56+
await mockApi.mockAgent.close()
57+
})
58+
59+
test('returns success when deploy stage succeeds', async () => {
60+
expect.assertions(3)
61+
62+
mockApi.interceptCloudflare(MOCK_API_PATH_DEPLOYMENTS, RESPONSE_DEPLOYMENTS)
63+
64+
const {deployment, status} = await statusCloudflareDeployment(API_ENDPOINT)
65+
66+
expect(status).toBe('success')
67+
expect(deployment.id).toMatchInlineSnapshot(
68+
`"206e215c-33b3-4ce4-adf4-7fc6c9b65483"`
69+
)
70+
expect(vi.mocked(sleep)).not.toHaveBeenCalled()
71+
})
72+
73+
test('polls until deploy stage succeeds', async () => {
74+
expect.assertions(2)
75+
76+
mockApi
77+
.interceptCloudflare(MOCK_API_PATH_DEPLOYMENTS, RESPONSE_DEPLOYMENTS_IDLE)
78+
.times(2)
79+
mockApi.interceptCloudflare(MOCK_API_PATH_DEPLOYMENTS, RESPONSE_DEPLOYMENTS)
80+
81+
const {status} = await statusCloudflareDeployment(API_ENDPOINT)
82+
83+
expect(status).toBe('success')
84+
expect(vi.mocked(sleep)).toHaveBeenCalledTimes(2)
85+
})
86+
87+
test.for([
88+
{stage: 'build', status: 'failure'},
89+
{stage: 'build', status: 'canceled'},
90+
{stage: 'deploy', status: 'active'}
91+
] satisfies {stage: LatestStage['name']; status: LatestStage['status']}[])(
92+
'returns $status immediately without polling ($stage stage)',
93+
async ({stage, status}) => {
94+
expect.assertions(2)
95+
96+
mockApi.interceptCloudflare(
97+
MOCK_API_PATH_DEPLOYMENTS,
98+
withLatestStage(stage, status)
99+
)
100+
101+
const result = await statusCloudflareDeployment(API_ENDPOINT)
102+
103+
expect(result.status).toBe(status)
104+
expect(vi.mocked(sleep)).not.toHaveBeenCalled()
105+
}
106+
)
107+
108+
test('polls while a non-deploy stage is active', async () => {
109+
expect.assertions(2)
110+
111+
mockApi.interceptCloudflare(
112+
MOCK_API_PATH_DEPLOYMENTS,
113+
withLatestStage('build', 'active')
114+
)
115+
mockApi.interceptCloudflare(MOCK_API_PATH_DEPLOYMENTS, RESPONSE_DEPLOYMENTS)
116+
117+
const {status} = await statusCloudflareDeployment(API_ENDPOINT)
118+
119+
expect(status).toBe('success')
120+
expect(vi.mocked(sleep)).toHaveBeenCalledTimes(1)
121+
})
122+
123+
test('throws when the api returns an error', async () => {
124+
expect.assertions(1)
125+
126+
mockApi.interceptCloudflare(
127+
MOCK_API_PATH_DEPLOYMENTS,
128+
{result: null, success: false, errors: [], messages: []},
129+
404
130+
)
131+
132+
await expect(
133+
statusCloudflareDeployment(API_ENDPOINT)
134+
).rejects.toThrowErrorMatchingInlineSnapshot(
135+
`[ParseError: A request to the Cloudflare API (https://api.cloudflare.com/client/v4/accounts/mock-cloudflare-account-id/pages/projects/mock-cloudflare-project-name/deployments) failed.]`
136+
)
137+
})
138+
})

__tests__/deploy/main.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest'
33

44
import type {MockApi} from '@/tests/helpers/api.js'
55

6-
import {execAsync} from '@/common/utils.js'
6+
import {execFileAsync} from '@/common/utils.js'
77
import {run} from '@/deploy/main.js'
88
import RESPONSE_DEPLOYMENTS from '@/responses/api.cloudflare.com/pages/deployments/deployments.response.json' with {type: 'json'}
99
import {MOCK_API_PATH_DEPLOYMENTS, setMockApi} from '@/tests/helpers/api.js'
@@ -31,7 +31,7 @@ describe('deploy', () => {
3131
describe(run, () => {
3232
describe('handles resolve', () => {
3333
beforeEach(() => {
34-
vi.mocked(execAsync).mockResolvedValue({
34+
vi.mocked(execFileAsync).mockResolvedValue({
3535
stdout: 'success',
3636
stderr: ''
3737
})

0 commit comments

Comments
 (0)