Skip to content

Commit 7ba744f

Browse files
authored
fix: remove shouldUnregister bug + CI pipeline restructure (#111)
* fix: remove shouldUnregister that silently broke create-listing form shouldUnregister: true stripped conditionally-rendered field values (duration, extendedBiddingTriggerWindow, extendedBiddingExtension) from the form data when their inputs unmounted, causing Zod validation to fail silently — the "Create Draft" button appeared to do nothing. * ci: build once + parallel test matrix + parallel frontend matrix Three structural improvements to the CI pipeline: 1. Build/test separation: the Build job compiles once and uploads the workspace as an artifact; the 10 test matrix entries download it and run with --no-build, eliminating 10 redundant full builds. 2. Test matrix display fix: the matrix uses if: always() so GitHub always expands it, showing proper "Test — Contracts" etc. names even on frontend-only PRs. A gate step prevents actual execution when there are no code changes or the build did not succeed. 3. Frontend parallelism: the sequential frontend job is replaced by a 5-entry matrix (shared, bidder, ops, seller, e2e) so all SPA builds and tests run in parallel instead of sequentially. * fix(ci): write tar archive to /tmp to avoid self-referential read tar -czf dotnet-build.tar.gz . creates the archive inside the directory being archived, causing "file changed as we read it" (exit code 1). Writing to /tmp first avoids the self-reference.
1 parent 91c0d66 commit 7ba744f

2 files changed

Lines changed: 80 additions & 65 deletions

File tree

.github/workflows/ci.yml

Lines changed: 80 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ env:
2424
DOTNET_CLI_TELEMETRY_OPTOUT: true
2525

2626
jobs:
27+
# ─── Path filtering ─────────────────────────────────────────────────
2728
changes:
2829
name: Detect changes
2930
runs-on: ubuntu-latest
@@ -37,13 +38,6 @@ jobs:
3738
uses: dorny/paths-filter@v3
3839
id: filter
3940
with:
40-
# The `code` filter is true when any file relevant to building or testing
41-
# the .NET solution has changed; the `frontend` filter is true when the
42-
# client/ SPA workspace changes. Doc-only PRs (e.g. README, docs/**, *.md,
43-
# LICENSE, CODE_OF_CONDUCT) match neither, and the build/test/frontend jobs
44-
# are skipped — the final `ci` aggregator job then succeeds so branch
45-
# protection still passes. A change to this workflow file trips both filters
46-
# so the full suite re-validates when CI itself changes.
4741
filters: |
4842
code:
4943
- 'src/**'
@@ -61,12 +55,12 @@ jobs:
6155
- 'client/**'
6256
- '.github/workflows/ci.yml'
6357
58+
# ─── .NET build (single, shared by all test runners) ────────────────
6459
build:
6560
name: Build
6661
needs: changes
6762
if: needs.changes.outputs.code == 'true'
6863
runs-on: ubuntu-latest
69-
7064
steps:
7165
- name: Checkout
7266
uses: actions/checkout@v4
@@ -100,10 +94,36 @@ jobs:
10094
path: ./publish
10195
retention-days: 5
10296

103-
integration-tests:
104-
name: Integration — ${{ matrix.name }}
105-
needs: changes
106-
if: needs.changes.outputs.code == 'true'
97+
- name: Archive build output for test runners
98+
run: |
99+
tar -czf /tmp/dotnet-build.tar.gz \
100+
--exclude='.git' \
101+
--exclude='node_modules' \
102+
--exclude='client' \
103+
--exclude='publish' \
104+
--exclude='docs' \
105+
--exclude='openspec' \
106+
--exclude='.github' \
107+
.
108+
mv /tmp/dotnet-build.tar.gz ./dotnet-build.tar.gz
109+
110+
- name: Upload build output
111+
uses: actions/upload-artifact@v4
112+
with:
113+
name: dotnet-build
114+
path: dotnet-build.tar.gz
115+
retention-days: 1
116+
117+
# ─── Per-BC integration tests (parallel matrix) ─────────────────────
118+
# if: always() ensures the matrix expands even when build is skipped
119+
# (e.g. frontend-only PRs), so each entry displays its proper name in
120+
# the GitHub checks UI instead of the raw "${{ matrix.name }}" template.
121+
# The gate step prevents actual test execution when there are no code
122+
# changes or the build did not succeed.
123+
test:
124+
name: Test — ${{ matrix.name }}
125+
needs: [changes, build]
126+
if: always()
107127
runs-on: ubuntu-latest
108128
strategy:
109129
fail-fast: false
@@ -141,29 +161,29 @@ jobs:
141161
trx: operations-tests.trx
142162

143163
steps:
144-
- name: Checkout
145-
uses: actions/checkout@v4
164+
- name: Gate — check for code changes and successful build
165+
id: gate
166+
if: needs.changes.outputs.code == 'true' && needs.build.result == 'success'
167+
run: echo "run=true" >> "$GITHUB_OUTPUT"
146168

147169
- name: Setup .NET
170+
if: steps.gate.outputs.run == 'true'
148171
uses: actions/setup-dotnet@v4
149172
with:
150173
dotnet-version: ${{ env.DOTNET_VERSION }}
151174

152-
- name: Cache NuGet packages
153-
uses: actions/cache@v4
175+
- name: Download build output
176+
if: steps.gate.outputs.run == 'true'
177+
uses: actions/download-artifact@v4
154178
with:
155-
path: ~/.nuget/packages
156-
key: ${{ runner.os }}-nuget-${{ hashFiles('**/Directory.Packages.props', '**/*.csproj') }}
157-
restore-keys: |
158-
${{ runner.os }}-nuget-
179+
name: dotnet-build
159180

160-
- name: Restore dependencies
161-
run: dotnet restore CritterBids.slnx
162-
163-
- name: Build
164-
run: dotnet build CritterBids.slnx --no-restore --configuration ${{ env.CONFIGURATION }}
181+
- name: Extract build output
182+
if: steps.gate.outputs.run == 'true'
183+
run: tar -xzf dotnet-build.tar.gz
165184

166185
- name: Test — ${{ matrix.name }}
186+
if: steps.gate.outputs.run == 'true'
167187
run: >
168188
dotnet test ${{ matrix.project }}
169189
--no-build --configuration ${{ env.CONFIGURATION }}
@@ -173,17 +193,32 @@ jobs:
173193
174194
- name: Upload test results
175195
uses: actions/upload-artifact@v4
176-
if: always()
196+
if: always() && steps.gate.outputs.run == 'true'
177197
with:
178-
name: integration-test-results-${{ matrix.name }}
198+
name: test-results-${{ matrix.name }}
179199
path: ./test-results
180200
retention-days: 5
181201

202+
# ─── Frontend (parallel per-app matrix) ─────────────────────────────
182203
frontend:
183-
name: Frontend
204+
name: Frontend — ${{ matrix.app }}
184205
needs: changes
185206
if: needs.changes.outputs.frontend == 'true'
186207
runs-on: ubuntu-latest
208+
strategy:
209+
fail-fast: false
210+
matrix:
211+
include:
212+
- app: shared
213+
check: typecheck
214+
- app: bidder
215+
check: build-test
216+
- app: ops
217+
check: build-test
218+
- app: seller
219+
check: build-test
220+
- app: e2e
221+
check: typecheck
187222
defaults:
188223
run:
189224
working-directory: client
@@ -201,64 +236,45 @@ jobs:
201236
- name: Install dependencies
202237
run: npm ci
203238

204-
- name: Typecheck shared (tsc strict)
205-
run: npx tsc --noEmit -p shared/tsconfig.json
206-
207-
- name: Build bidder (tsc strict + vite build)
208-
run: npm run build -w @critterbids/bidder
209-
210-
- name: Test bidder (vitest)
211-
run: npm test -w @critterbids/bidder
212-
213-
- name: Build ops (tsc strict + vite build)
214-
run: npm run build -w @critterbids/ops
215-
216-
- name: Test ops (vitest)
217-
run: npm test -w @critterbids/ops
218-
219-
- name: Build seller (tsc strict + vite build)
220-
run: npm run build -w @critterbids/seller
239+
- name: Typecheck (${{ matrix.app }})
240+
if: matrix.check == 'typecheck'
241+
run: npx tsc --noEmit -p ${{ matrix.app }}/tsconfig.json
221242

222-
- name: Test seller (vitest)
223-
run: npm test -w @critterbids/seller
243+
- name: Build (${{ matrix.app }})
244+
if: matrix.check == 'build-test'
245+
run: npm run build -w @critterbids/${{ matrix.app }}
224246

225-
# The Playwright e2e workspace member (client/e2e) is deliberately NOT run here
226-
# (M8-S7 recorded deferral): it needs the full Aspire-orchestrated stack — Postgres,
227-
# RabbitMQ, the API host, and the bidder dev server — live, which is its own piece of
228-
# CI infrastructure work, not a step addition. It runs locally pre-merge; see
229-
# client/e2e/README.md. Its tsconfig still type-checks against the shared strict base.
230-
- name: Typecheck e2e (tsc strict, no run)
231-
run: npm run typecheck -w @critterbids/e2e
247+
- name: Test (${{ matrix.app }})
248+
if: matrix.check == 'build-test'
249+
run: npm test -w @critterbids/${{ matrix.app }}
232250

233-
# Aggregator job suitable for use as the single required status check in
234-
# branch protection. A dependent job result of `success` or `skipped` is
235-
# acceptable (a job skips when its path filter did not match — e.g. a
236-
# frontend-only PR skips the .NET jobs, a backend-only PR skips Frontend,
237-
# a doc-only PR skips all). The aggregator fails only when a job that
251+
# ─── Aggregator (single required status check) ─────────────────────
252+
# A dependent job result of `success` or `skipped` is acceptable.
253+
# The `test` matrix uses `if: always()` so its result is `success`
254+
# (all steps gated) even when skipped — build failures are caught
255+
# via BUILD_RESULT. The aggregator fails only when a job that
238256
# actually ran reports `failure` or `cancelled`.
239257
ci:
240258
name: CI
241-
needs: [ changes, build, integration-tests, frontend ]
259+
needs: [ changes, build, test, frontend ]
242260
if: always()
243261
runs-on: ubuntu-latest
244262
steps:
245263
- name: Verify required jobs
246264
env:
247265
BUILD_RESULT: ${{ needs.build.result }}
248-
INTEGRATION_RESULT: ${{ needs.integration-tests.result }}
266+
TEST_RESULT: ${{ needs.test.result }}
249267
FRONTEND_RESULT: ${{ needs.frontend.result }}
250268
run: |
251269
set -euo pipefail
252270
253271
failed=0
254272
for entry in \
255273
"build:${BUILD_RESULT}" \
256-
"integration-tests:${INTEGRATION_RESULT}" \
274+
"test:${TEST_RESULT}" \
257275
"frontend:${FRONTEND_RESULT}"; do
258276
name="${entry%%:*}"
259277
result="${entry##*:}"
260-
# `success` and `skipped` pass; only a job that ran and did not
261-
# succeed (failure/cancelled) blocks the aggregator.
262278
if [ "${result}" = "failure" ] || [ "${result}" = "cancelled" ]; then
263279
echo "::error::Required job '${name}' did not succeed (result: ${result})"
264280
failed=1

client/seller/src/listings/CreateListingPage.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ export function CreateListingPage() {
4949
} = useForm<CreateDraftFormValues>({
5050
resolver,
5151
defaultValues,
52-
shouldUnregister: true,
5352
});
5453

5554
const format = watch("format");

0 commit comments

Comments
 (0)