Skip to content

✨ Add Windows test workflow for PR CI#6687

Merged
czunker merged 4 commits intomainfrom
czunker/pr-test-windows-workflow
Mar 3, 2026
Merged

✨ Add Windows test workflow for PR CI#6687
czunker merged 4 commits intomainfrom
czunker/pr-test-windows-workflow

Conversation

@czunker
Copy link
Copy Markdown
Contributor

@czunker czunker commented Feb 26, 2026

Summary

Test plan

  • Verify the workflow runs successfully on Windows
  • Confirm it picks up //go:build windows test files

🤖 Generated with Claude Code

Extract the Windows test workflow from #6474 to enable running
Windows-specific tests (e.g. registry-based detection) in CI.

See: #6474

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous version ran all tests in the services package on Windows,
which included Linux/macOS tests that fail due to path separators and
symlinks. Now dynamically discovers packages containing _windows_test.go
files and only runs those.

See: #6474

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@czunker czunker marked this pull request as ready for review February 26, 2026 10:43
czunker added a commit that referenced this pull request Feb 26, 2026
Add _windows_test.go files to exercise native Windows code paths
(registry API, WMI, IP Helper API, fsutil) that were previously
untested. These tests complement the Windows CI workflow from PR #6687.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@chris-rock
Copy link
Copy Markdown
Member

/review

Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review bot left a comment

Choose a reason for hiding this comment

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

New Windows CI workflow is well-structured but has a path construction bug and a minor robustness issue.

- Use relative paths for package discovery (DirectoryName returns absolute)
- Filter empty strings from package split to avoid ghost arguments
- Propagate go test exit code through Tee-Object
- Add -count=1 to disable test caching in CI

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@czunker
Copy link
Copy Markdown
Contributor Author

czunker commented Mar 2, 2026

/review

1 similar comment
@czunker
Copy link
Copy Markdown
Contributor Author

czunker commented Mar 2, 2026

/review

Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review bot left a comment

Choose a reason for hiding this comment

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

Well-structured Windows CI workflow with one script injection concern worth addressing.

@czunker
Copy link
Copy Markdown
Contributor Author

czunker commented Mar 3, 2026

/review

@mondoo-code-review
Copy link
Copy Markdown

Unable to complete the code review.

Reason: An internal error occurred during the review.

Error details
code-reviewer: claude failed: exit status 1
output: {"type":"result","subtype":"success","is_error":true,"duration_ms":240,"duration_api_ms":0,"num_turns":1,"result":"Credit balance is too low","stop_...

You can try /review again or reduce the PR size.

@mondoo-code-review mondoo-code-review bot dismissed their stale review March 3, 2026 07:05

Dismissed: review could not be completed

Pass test packages via environment variable instead of direct interpolation
to prevent potential script injection. Remove push trigger to match the
PR-only intent of the workflow filename.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review bot left a comment

Choose a reason for hiding this comment

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

Well-structured Windows CI workflow with pinned actions and proper permissions; minor suggestions on test package discovery.

@czunker
Copy link
Copy Markdown
Contributor Author

czunker commented Mar 3, 2026

/review

Copy link
Copy Markdown

@mondoo-code-review mondoo-code-review bot left a comment

Choose a reason for hiding this comment

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

Well-structured Windows CI workflow with pinned actions and minimal permissions; no significant issues found.

$rel = ($_.DirectoryName -replace '\\','/').Replace($repoRoot + '/', '')
"./$rel/..."
} |
Sort-Object -Unique
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 suggestion — Appending /... makes each entry match the package and all sub-packages. If _windows_test.go files exist in both providers/os/a/ and providers/os/a/b/, Sort-Object -Unique won't deduplicate them (different strings), so tests in b will be run twice — once via ./providers/os/a/... and once via ./providers/os/a/b/.... go test tolerates this (no incorrect results), but you could avoid the redundant work by dropping the /... suffix (i.e. use ./$rel) since you're already enumerating every directory that contains test files.

@czunker czunker merged commit 7834a12 into main Mar 3, 2026
11 checks passed
@czunker czunker deleted the czunker/pr-test-windows-workflow branch March 3, 2026 12:55
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants