Skip to content

spa: Fix TestURLToRelAcceptsSafeRelativePath for Windows cross-platform compatibility - #7274

Open
nikunjkumar05 wants to merge 1 commit into
kubernetes-sigs:mainfrom
nikunjkumar05:fix-test
Open

spa: Fix TestURLToRelAcceptsSafeRelativePath for Windows cross-platform compatibility#7274
nikunjkumar05 wants to merge 1 commit into
kubernetes-sigs:mainfrom
nikunjkumar05:fix-test

Conversation

@nikunjkumar05

@nikunjkumar05 nikunjkumar05 commented Aug 15, 2026

Copy link
Copy Markdown

Summary

This PR fixes TestURLToRelAcceptsSafeRelativePath test failure on Windows by making the test expectation OS-aware using runtime.GOOS.

Changes

  • Fixed backend/pkg/spa/pathSafety_internal_test.go to handle Windows path separator conversion
  • Added runtime import for OS detection
  • Made test expect "headlamp\\assets\\main.js" on Windows and "headlamp/assets/main.js" on Linux/macOS

Steps to Test

  1. Run go test -count=1 ./pkg/spa on Windows
  2. Verify TestURLToRelAcceptsSafeRelativePath passes

Notes for the Reviewer

  • This is a test-only change - no production code modified
  • The urlToRel function correctly produces OS-specific paths via filepath.FromSlash() - the test expectation was written for Linux/macOS format only

@kubernetes-prow kubernetes-prow Bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 15, 2026
@kubernetes-prow
kubernetes-prow Bot requested review from ashu8912 and gambtho August 15, 2026 22:07
@kubernetes-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: nikunjkumar05
Once this PR has been reviewed and has the lgtm label, please assign joaquimrocha for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates the SPA path-safety test to accept OS-specific path separators.

Changes:

  • Adds Windows OS detection.
  • Uses backslashes in the Windows expectation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@illume illume left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the contribution.

The backend test job in CI is failing. Run cd backend && go test ./... to reproduce the errors locally.

How to run the backend tests

Run cd backend && go test ./... to see all failures. Fix the failing tests and commit the result.

@kubernetes-prow kubernetes-prow Bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 16, 2026
@nikunjkumar05

Copy link
Copy Markdown
Author

@illume thanks for review. Resolved the error.

@nikunjkumar05
nikunjkumar05 requested a review from illume August 16, 2026 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants