Skip to content

fix: webserver test skips when frontend assets not built#204

Merged
spboyer merged 2 commits into
mainfrom
squad/fix-webserver-ci-test
Apr 21, 2026
Merged

fix: webserver test skips when frontend assets not built#204
spboyer merged 2 commits into
mainfrom
squad/fix-webserver-ci-test

Conversation

@spboyer
Copy link
Copy Markdown
Member

@spboyer spboyer commented Apr 21, 2026

Problem

TestIndexHTMLReferencesExistingAssets fails on Windows CI because npm run build only runs on the ubuntu-latest matrix. The test's skip guard only checked whether the assets/ directory existed — not whether it contained actual JS/CSS bundles. If the directory exists but is empty (cache artefact, partial build), the test proceeds and fails when the SPA fallback serves index.html instead of the expected asset content types.

Fix

After confirming assets/ exists, the test now also verifies it contains at least one .js or .css file. If not, it skips with a descriptive message pointing at npm run build.

Verification

  • go test -count=1 -v ./internal/webserver/... — all pass
  • go test ./... — full suite green
  • go vet ./... — clean

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

The TestIndexHTMLReferencesExistingAssets test only checked whether the
assets/ directory existed, but not whether it contained actual JS/CSS
bundles. On Windows CI (and any env where npm run build hasn't run),
the directory could exist empty or with non-bundle files, causing the
test to proceed and fail when the SPA fallback served index.html
instead of the expected asset content types.

Now the test also verifies that assets/ contains at least one .js or
.css file before proceeding, and skips with a clear message otherwise.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 21, 2026 18:07
@github-actions github-actions Bot enabled auto-merge (squash) April 21, 2026 18:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes Windows CI flakiness in internal/webserver by making the TestIndexHTMLReferencesExistingAssets skip guard verify that frontend bundles are actually present, not just that the assets/ directory exists.

Changes:

  • Read dist/assets directory entries and fail fast on unexpected FS errors.
  • Skip TestIndexHTMLReferencesExistingAssets when dist/assets exists but contains no .js/.css bundles, with a message pointing to npm run build.
Show a summary per file
File Description
internal/webserver/server_test.go Strengthens the test precondition for embedded frontend assets to avoid false failures when assets aren’t built.

Copilot's findings

Comments suppressed due to low confidence (1)

internal/webserver/server_test.go:186

  • The skip check only requires either a .js or .css file. If a partial/cached build leaves just one of those (or only non-bundle files), this test will still run and then fail later when a referenced asset is missing and served by the SPA fallback. Since index.html references both JS and CSS bundles, consider tracking hasJS and hasCSS (similar to TestEmbeddedAssetsDirectoryContainsBundles) and skipping unless both are present.
	var hasBundles bool
	for _, e := range entries {
		if !e.IsDir() {
			ext := filepath.Ext(e.Name())
			if ext == ".js" || ext == ".css" {
				hasBundles = true
  • Files reviewed: 1/1 changed files
  • Comments generated: 1

Comment thread internal/webserver/server_test.go
- Fix 'artefact' → 'artifact' misspelling in webserver test
- Use runtime.GOOS to pick platform-correct absolute path in suggest test

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@spboyer spboyer merged commit 0cfa604 into main Apr 21, 2026
6 checks passed
@spboyer spboyer deleted the squad/fix-webserver-ci-test branch April 21, 2026 18:21
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