Skip to content

test: cover Windows drive-relative resolution errors#71

Merged
hyfdev merged 1 commit into
mainfrom
agent/test-windows-drive-relative-errors
Jul 19, 2026
Merged

test: cover Windows drive-relative resolution errors#71
hyfdev merged 1 commit into
mainfrom
agent/test-windows-drive-relative-errors

Conversation

@hyfdev

@hyfdev hyfdev commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • add a deterministic Windows-only regression test for embedded-NUL drive-relative paths
  • require try_absolutize and both target/base roles of try_relative to preserve the native error kind and raw OS error, while the strict APIs panic
  • require the test to register and execute in both default and cached_current_dir CI configurations, and record the contract in the testing strategy

Why

Windows drive-relative ambient resolution delegates to std::path::absolute. Existing coverage exercised successful invalid-wide resolution and Unix cwd failures, but not an error from this Windows-specific branch. A regression could therefore bypass the native per-drive resolution path or mishandle its error without being detected.

An embedded NUL is deterministic under the pinned Rust 1.97.0 standard library: conversion to the WinAPI input buffer returns InvalidInput before GetFullPathNameW, so the test does not depend on drive mappings, filesystem contents, or runner cwd state.

Impact

This is a test-only contract change. Production code, dependencies, benchmarks, and allocation baselines are unchanged.

Validation

  • cargo fmt --all --check
  • default and cached_current_dir workspace test suites
  • cargo clippy --locked --workspace --all-targets --all-features -- -D warnings
  • RUSTDOCFLAGS="-D warnings" cargo doc --locked -p sugar_path --all-features --no-deps
  • MSVC and GNU Windows library plus test-metadata compilation in both feature configurations
  • independent adversarial review with no findings
  • PR Test run: all seven jobs passed, with the new test executing successfully on native Windows MSVC in both feature configurations

@hyfdev

hyfdev commented Jul 19, 2026

Copy link
Copy Markdown
Owner Author

Adversarial review conclusion

Reviewed PR #71 (head 928118e5) as an adversarial pass over the test-only Windows drive-relative error coverage.

Verdict: approve — no open findings.

Scope checked

  • .agents/docs/testing-strategy.md
  • .github/workflows/test.yaml
  • tests/windows_drive_relative_errors.rs
  • surrounding production path for Windows drive-relative ambient resolution and try_* / strict API error contracts

Assessment

This fills a real branch gap: ambient drive-relative resolution goes through std::path::absolute, and the new integration test forces that path with an embedded NUL so failure does not depend on drive maps, filesystem state, or cwd. Against the pinned Rust 1.97.0 std, interior NULs are rejected with ErrorKind::InvalidInput before GetFullPathNameW, so the claimed determinism holds. Production try_absolutize / try_relative propagate via ?; strict APIs panic through .expect(...). CI registers the exact test name on the Windows host list for both default and cached_current_dir listing gates, and the testing-strategy note matches the wiring.

No correctness, flakiness, incomplete contract coverage, or documentation mismatches found.

@hyfdev
hyfdev marked this pull request as ready for review July 19, 2026 04:02
Copilot AI review requested due to automatic review settings July 19, 2026 04:02
@hyfdev
hyfdev merged commit 0458552 into main Jul 19, 2026
7 checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot mentioned this pull request Jul 19, 2026
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.

2 participants