Skip to content

Five tests fail on Windows (POSIX file-mode assertions + a path assumption) — never run there until now #897

Description

@ako

Summary

Five tests fail on Windows. They are not new — they have simply never been run there, because CI was ubuntu-only until #896 added a Windows/macOS job. macOS passes all five.

Found by the tunnel-seam-cross-platform job in run 31820311381. That job is now scoped with -run to the tunnel-seam tests it was added for, so #896 is not blocked; widening it back to the full packages is this issue's job.

Failures

Test File Failure
TestSnapshotStorageFormat_RestoresV2AfterConversion cmd/mxcli/docker/check_test.go:99 restored .mpr mode = 666, want 0600
TestUpdateWidgetsPathArg_AbsolutizesBareFilename cmd/mxcli/docker/check_test.go:232 updateWidgetsPathArg(abs) = "D:\\proj\\app.mpr", want it unchanged
TestKeyStore_FileMode0600 cmd/mxcli/tunnelhub/keys_test.go:113 key file mode = 666, want 600
TestKeyStore_RejectsTooOpenFile cmd/mxcli/tunnelhub/keys_test.go:123 a world-readable key file should be refused
TestNew_ResolvesSpecs cmd/mxcli/tunnelhub/audit/audit_test.go:89 audit file mode = 666, want 600 (plus a TempDir RemoveAll failure — the audit file is still open when cleanup runs)

Cause

Four of the five are the same thing: Windows does not implement POSIX permission bits. os.Chmod only toggles the read-only attribute, and Stat().Mode().Perm() reports 0666 for any writable file. So 0600 assertions can never hold, and a "is this file world-readable?" check can never be satisfied.

TestUpdateWidgetsPathArg_AbsolutizesBareFilename is different and needs a real look: it is path-separator semantics, and it may be a genuine bug in updateWidgetsPathArg rather than a bad assertion.

Why these are not all "just test bugs"

TestKeyStore_RejectsTooOpenFile is asserting a security control: the hub key store refuses to load a key file that others can read. On Windows that check is not merely untested — it cannot pass, which suggests NewKeyStoreFile would refuse to load its own key file there. Worth confirming before deciding the fix: a t.Skip would be wrong if the production code is what's broken.

Mitigating: cmd/mxcli/tunnelhub (key store, audit sink) is only reachable from mxcli tunnel-hub, which as of #896 is Linux-only. So the tunnelhub/audit failures are testing code that can no longer run on Windows at all. cmd/mxcli/docker's two failures are in genuinely cross-platform paths (docker check, update-widgets) and matter more.

Suggested resolution

  1. Decide per test whether the assertion or the production code is wrong — do not blanket-t.Skip the file-mode tests without checking whether the guarded code misbehaves on Windows first.
  2. For assertions that are genuinely POSIX-only, gate on runtime.GOOS with a comment saying why, rather than weakening the check on Linux where it is load-bearing.
  3. Fix the TempDir RemoveAll leak in audit_test.go — the sink is not closed before cleanup, which is a real file-handle bug that only Windows's mandatory locking surfaces.
  4. Investigate updateWidgetsPathArg on Windows paths properly.
  5. Then widen the tunnel-seam-cross-platform job back to the full packages (drop the -run filter in .github/workflows/push-test.yml) so Windows stays covered.

Notes

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions