test: add case for #65 #100
Conversation
WalkthroughThis update introduces enhanced Windows-specific symlink testing and utility functions. The Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant SymlinkFixture
participant WindowsUtils
TestRunner->>SymlinkFixture: prepare_symlinks()
SymlinkFixture-->>TestRunner: SymlinkFixturePaths or skips if no permissions
TestRunner->>WindowsUtils: get_dos_device_path(path)
WindowsUtils-->>TestRunner: DOS device path or error
TestRunner->>TestRunner: Run symlink resolution tests
TestRunner->>TestRunner: Run unsupported DOS device path tests (Windows only)
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 68d0f0d in 2 minutes and 56 seconds. Click for details.
- Reviewed
283lines of code in6files - Skipped
0files when reviewing. - Skipped posting
9draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/tests/symlink.rs:102
- Draft comment:
In the prepare_symlinks function, the fallback on permission failure prints and returns Ok(None). Consider logging more structured error info rather than swallowing the error completely. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
2. src/tests/symlink.rs:121
- Draft comment:
The test iterates over many symlink resolution scenarios. It may be beneficial to document or group the chained cases for clarity. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
3. src/tests/windows.rs:51
- Draft comment:
When replacing '\?' with '\.', ensure that the intended OS behavior is properly documented, since many IO APIs expect '\.' paths. A note explaining why this replacement is necessary is useful. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that the intended OS behavior is documented and to explain why the replacement is necessary. This violates the rule against asking the author to confirm their intention or to explain their changes. The comment does not provide a specific code suggestion or ask for a specific test to be written.
4. src/tests/windows.rs:71
- Draft comment:
The test_get_dos_device_path function only tests the positive (canonicalization) path. Consider adding tests for error scenarios (e.g., invalid mount points) to enhance coverage. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
5. src/tests/symlink.rs:36
- Draft comment:
The init() function creates symlink fixtures then immediately removes the directory. This appears to be a permissions check, but the intent isn’t immediately clear. Please add a clarifying comment. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. src/tests/symlink.rs:189
- Draft comment:
The expected error in test_unsupported_targets relies on dos_device_temp_path.parent().unwrap().parent().unwrap(), which assumes a specific directory structure. Please clarify this assumption with a comment or compute the expected path more robustly. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment raises a valid concern about test fragility. However, the code already has an explanatory comment about why this path computation works. The test is dealing with Windows-specific DOS device paths which inherently require certain directory structures. Making this more "robust" would likely overcomplicate the test without adding value. The comment identifies a real potential issue with test maintainability. A more robust path computation could prevent test breakage if the directory structure changes. Given the Windows-specific nature and the existing explanatory comment, the current implementation is a reasonable tradeoff between test clarity and robustness. Making it more robust would add complexity for little benefit. Delete the comment. The code already explains the directory structure assumption, and making the path computation more robust would overcomplicate this Windows-specific test case.
7. src/tests/windows.rs:40
- Draft comment:
In get_dos_device_path, the code panics if the provided path does not have a root. Consider returning an error instead of panicking for better error handling in non-test contexts. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a Windows-specific function dealing with DOS device paths, which by definition must have roots. The panic serves as an assertion of a fundamental precondition. The function name and context make it clear this is for Windows paths only. Converting to error handling might actually make the API less clear by suggesting rootless paths are valid input. The comment has a point about defensive programming. In a public API, panicking is generally considered less idiomatic than returning errors in Rust. However, this is a specialized Windows path conversion function where a rootless path is a programmer error - it's reasonable to panic on fundamentally invalid input rather than treating it as a runtime error case. The panic is an appropriate design choice here as it enforces a fundamental precondition. Converting it to error handling would not improve the API.
8. src/tests/symlink.rs:68
- Draft comment:
Typo: In the comment on line 68, 'driver letter' should be corrected to 'drive letter' to accurately describe the Windows drive letter concept. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While the typo fix is technically correct, it's just a minor spelling error in a comment that doesn't affect code functionality. The meaning is still clear even with the typo. The rules state not to make purely informative comments and to only comment if there's clearly a code change required. The typo could potentially confuse readers who are not familiar with Windows terminology. Clear documentation is important for maintainability. While clear documentation is valuable, this is an extremely minor typo that doesn't significantly impact understanding. The rules explicitly state not to make purely informative comments. The comment should be deleted as it's a purely informative comment about a minor typo that doesn't affect code functionality.
9. src/tests/windows.rs:47
- Draft comment:
Line 47: The use of a raw string literal (r"") with a single trailing backslash may cause issues because raw strings in Rust cannot end with a backslash. Consider using a normal string literal, e.g., "\" instead, to ensure the backslash is correctly represented. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% Raw string literals in Rust do have special handling for backslashes, but in this caser"\"is actually valid syntax and works correctly. The comment is incorrect in stating that raw strings cannot end with a backslash. While"\\"would also work, there's no actual issue to fix here. The test at the bottom of the file would fail if this wasn't working correctly. Could there be platform-specific edge cases where raw string literal backslash handling behaves differently? Could the suggestion make the code more readable or maintainable? The raw string syntax is well-defined in Rust and behaves consistently across platforms. Both syntaxes are equally readable and maintainable. The comment should be deleted as it incorrectly claims there is an issue with the code, when the current implementation is perfectly valid and working.
Workflow ID: wflow_wGHdzOazB18dh5zW
You can customize 
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/tests/symlink.rs (1)
53-55: 🛠️ Refactor suggestionAvoid
unwrap()– propagate canonicalisation errors instead
canonicalize()can legitimately fail (e.g. fixture path moved or has wrong case on Windows).
Usingunwrap()will panic and abort the whole test run, defeating the graceful-skip logic you added above.- symlink(dirname.join("../lib").canonicalize().unwrap(), temp_path.join("lib"), FileType::Dir)?; - symlink(dirname.join("..").canonicalize().unwrap(), temp_path.join("this"), FileType::Dir)?; + symlink(dirname.join("../lib").canonicalize()?, temp_path.join("lib"), FileType::Dir)?; + symlink(dirname.join("..").canonicalize()?, temp_path.join("this"), FileType::Dir)?;Repeating the pattern elsewhere in this function keeps error handling consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
Cargo.toml(1 hunks)README.md(1 hunks)src/tests/mod.rs(1 hunks)src/tests/symlink.rs(4 hunks)src/tests/windows.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/tests/windows.rs (1)
src/tests/mod.rs (1)
fixture_root(32-34)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (8)
src/tests/mod.rs (1)
26-26: Good addition of the Windows-specific test module.Adding the
windowsmodule is appropriate for handling the Windows-specific path conversion utilities that support the symlink tests for DOS device paths. This aligns well with the PR objectives to improve handling of unsupported DOS device paths in symlinks.Cargo.toml (1)
79-79: Good dependency addition for Windows API support.Adding the
windows-syscrate with the appropriate features (Win32_FoundationandWin32_Storage_FileSystem) is necessary to support the Windows-specific path conversion utilities in the new test module. The dependency version is current and the feature selection is minimal and appropriate for the implemented functionality.README.md (1)
166-166: Clear documentation improvement for the symlinks option.The addition of "if possible" with a link to the implementation details improves the documentation by clarifying that symlink resolution is conditional. This aligns with the PR objective of not following symlinks to DOS device paths on Windows, and provides users with an easy way to discover the specific conditions under which symlinks are resolved.
src/tests/windows.rs (5)
1-12: Good module organization and appropriate imports.The imports are well-organized and include the necessary Windows-specific functionality for UTF-16 encoding/decoding (
OsStrExt,OsStringExt) and the Windows API functions fromwindows-sysneeded for volume path operations.
13-35: Well-implemented Windows API wrapper with proper error handling.The
volume_name_from_mount_pointfunction is a well-documented wrapper around the Windows APIGetVolumeNameForVolumeMountPointW. It correctly handles:
- UTF-16 encoding required by the Windows API
- Buffer allocation with a reasonable size
- Proper error handling using a custom error type
- NULL termination of strings
- Safe conversion back to Rust's
OsStringThe implementation follows Windows API best practices.
37-62: Good implementation of DOS device path conversion.The
get_dos_device_pathfunction correctly:
- Validates that the input path has a root component
- Extracts and prepares the root with a trailing backslash as required by the API
- Modifies the volume name prefix from
\\?\to\\.\for better compatibility- Constructs the new path by combining the volume name with the remaining components
The implementation is thorough and handles the nuances of Windows path formats.
64-69: Good custom error type implementation.The
Win32Errorstruct is a well-designed error type that:
- Derives appropriate traits (
Debug,Clone,PartialEq)- Uses thiserror for clean error message formatting
- Includes the Windows error code for detailed diagnostics
- Has appropriate documentation referencing the Microsoft docs
This provides a clean error handling mechanism for the Windows API calls.
71-86: Comprehensive test case for path conversion.The test function validates that the DOS device path conversion works correctly by:
- Starting with a fixture root path
- Converting it to a DOS device path
- Canonicalizing both paths
- Verifying that they canonicalize to the same path
The debug prints are helpful for diagnosing issues during test runs, and the test validates the core functionality needed for the symlink tests.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/tests/windows.rs (1)
42-42: Return an error instead of asserting to avoid panicsThe function panics if the provided path doesn't have a root. For better error handling, especially in non-test contexts, consider returning an error instead.
Replace the assertion with:
- assert!(path.has_root(), "Expected a path with a root"); + if !path.has_root() { + return Err(Win32Error { error_code: 87 }); // ERROR_INVALID_PARAMETER + }
🧹 Nitpick comments (2)
src/tests/windows.rs (2)
51-56: Consider a more explicit approach for path prefix modificationWhile manually modifying the UTF-16 encoded string works, a more explicit approach might be clearer:
- if volume_name_root.starts_with(&[92, 92, 63, 92] /* \\?\ */) { - // Replace \\?\ with \\.\ - // While both is a valid DOS device path, "\\?\" won't be accepted by most of the IO operations. - volume_name_root[2] = u16::from(b'.'); - } + let prefix_query = [92, 92, 63, 92]; // \\?\ + let prefix_device = [92, 92, 46, 92]; // \\.\ + + if volume_name_root.starts_with(&prefix_query) { + // Replace \\?\ with \\.\ + // While both is a valid DOS device path, "\\.\\" is more compatible with IO operations. + for i in 0..prefix_device.len() { + volume_name_root[i] = prefix_device[i]; + } + }
72-87: Test function validates core functionalityThe test ensures that the DOS device path conversion produces a path that canonicalizes to the same result as the original path, which is the key property we want to verify.
Consider adding:
- Additional test cases with different path types (e.g., network paths, subst drives)
- Error case testing to verify error handling
- Making debug prints conditional with
#[cfg(test)]or similar to avoid cluttering test output
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/tests/symlink.rs(4 hunks)src/tests/windows.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/tests/symlink.rs
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/tests/windows.rs (1)
src/tests/mod.rs (1)
fixture_root(32-34)
🪛 GitHub Check: Codacy Static Code Analysis
src/tests/windows.rs
[notice] 26-26: src/tests/windows.rs#L26
Detected 'unsafe' usage, please audit for secure usage
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test (windows-latest)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
src/tests/windows.rs (3)
18-37: Function handles Windows API calls correctlyThe implementation of
volume_name_from_mount_pointproperly handles UTF-16 encoding/decoding for Windows API compatibility. The unsafe block is necessary for the Windows API call and is documented appropriately with the safety comment.Consider making the buffer size more robust by:
- Using a constant from Windows API if available
- Adding a fallback mechanism if the initial buffer is too small (though 64 chars should be sufficient for volume GUIDs)
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[notice] 26-26: src/tests/windows.rs#L26
Detected 'unsafe' usage, please audit for secure usage
64-69: Good error handling with Win32ErrorThe error type implementation using
thiserroris clean and provides good context for Windows API errors.
26-36: Ensure thread safety in Windows API callsThe unsafe block is necessary for the Windows API call, but it's important to ensure thread safety.
The
GetVolumeNameForVolumeMountPointWfunction appears to be thread-safe based on Windows documentation, but it's good to verify this. Would you like me to generate a script to check the documentation for this API call?🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[notice] 26-26: src/tests/windows.rs#L26
Detected 'unsafe' usage, please audit for secure usage
chenxinyanc
force-pushed
the
xinyan-dos-device-path-ut
branch
from
c0e7bcd to
5457f7b
Compare
CodSpeed Performance ReportMerging #100 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/tests/symlink.rs (1)
46-62:⚠️ Potential issueImprove canonicalization error handling
First canonicalization error is properly propagated, but subsequent ones are unwrapped and can panic.
symlink( dirname.join("../lib/index.js").canonicalize()?, temp_path.join("index.js"), FileType::File, )?; -symlink(dirname.join("../lib").canonicalize().unwrap(), temp_path.join("lib"), FileType::Dir)?; -symlink(dirname.join("..").canonicalize().unwrap(), temp_path.join("this"), FileType::Dir)?; +symlink(dirname.join("../lib").canonicalize()?, temp_path.join("lib"), FileType::Dir)?; +symlink(dirname.join("..").canonicalize()?, temp_path.join("this"), FileType::Dir)?;
♻️ Duplicate comments (3)
src/tests/windows.rs (1)
39-43:⚠️ Potential issueThe
get_dos_device_pathfunction can panicThe function currently panics if the provided path doesn't have a root. Consider returning an error instead to avoid panics in non-test contexts, as suggested in the previous review.
pub fn get_dos_device_path<P: AsRef<Path>>(path: P) -> Result<PathBuf, Win32Error> { let path = path.as_ref(); - assert!(path.has_root(), "Expected a path with a root"); + if !path.has_root() { + return Err(Win32Error { error_code: 87 }); // ERROR_INVALID_PARAMETER + }src/tests/symlink.rs (2)
63-80:⚠️ Potential issueHandle DOS device path conversion errors
The
unwrap()call can panic if DOS device path conversion fails, as noted in previous review comments.#[cfg(target_family = "windows")] { // Ideally we should point to a Volume that does not have a drive letter. // However, it's not trivial to create a Volume in CI environment. // Here we are just picking up any Volume, as resolver itself is not calling `fs::canonicalize`, // which potentially can resolve the Volume GUID into driver letter whenever possible. - let dos_device_temp_path = get_dos_device_path(temp_path).unwrap(); + let dos_device_temp_path = match get_dos_device_path(temp_path) { + Ok(path) => path, + Err(err) => { + println!("Skipped DOS device path symlinks: {err}"); + return Ok(()); + } + };
188-195:⚠️ Potential issueHandle DOS device path conversion errors in the test
Same issue as in
create_symlinks- theunwrap()can panic if DOS device path conversion fails.-let dos_device_temp_path = get_dos_device_path(&temp_path).unwrap(); +let dos_device_temp_path = match get_dos_device_path(&temp_path) { + Ok(path) => path, + Err(err) => { + println!("Skipped DOS device path test: {err}"); + return; + } +};
🧹 Nitpick comments (1)
src/tests/windows.rs (1)
11-37: Good implementation of Windows volume path conversionThe function properly handles UTF-16 encoding/decoding required for Windows API calls and includes appropriate error handling with the Windows error code.
However, the unsafe code block should include more detailed safety comments explaining preconditions and invariants.
// SAFETY: Win32 API call unsafe { // https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getvolumenameforvolumemountpointw + // SAFETY: We ensure mount_point is null-terminated UTF-16, and the buffer is properly sized. + // GetVolumeNameForVolumeMountPointW will not write beyond BUFFER_SIZE if properly null-terminated. if GetVolumeNameForVolumeMountPointW(mount_point.as_ptr(), buffer.as_mut_ptr(), BUFFER_SIZE) == 0 {🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[notice] 26-26: src/tests/windows.rs#L26
Detected 'unsafe' usage, please audit for secure usage
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/tests/symlink.rs(4 hunks)src/tests/windows.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/tests/windows.rs (2)
src/tests/symlink.rs (1)
test(118-159)src/tests/mod.rs (1)
fixture_root(32-34)
🪛 GitHub Check: Codacy Static Code Analysis
src/tests/windows.rs
[notice] 26-26: src/tests/windows.rs#L26
Detected 'unsafe' usage, please audit for secure usage
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test (windows-latest)
- GitHub Check: Benchmark
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (8)
src/tests/windows.rs (4)
1-10: Well-organized Windows-specific importsThe conditional compilation directive correctly scopes these imports to Windows targets, and the import selection covers all necessary components for Windows path handling and UTF-16 string conversion.
50-62: Well-implemented DOS device path conversion logicThe modification of the path prefix from
\\?\to\\.\is a good practice for ensuring compatibility with most IO operations. The code correctly extends the device path with the remaining components.
64-69: Good error type implementationThe
Win32Errortype is well-designed, providing useful error information with hex formatting for error codes and proper documentation linking to Microsoft's error code reference.
71-87: Well-structured test with good verification approachThe test effectively verifies that the DOS device path conversion works correctly by comparing canonicalized paths. The diagnostic println statements are helpful for debugging.
src/tests/symlink.rs (4)
1-2: Conditionally import Windows-specific functionsGood use of conditional compilation for Windows-specific imports.
89-115: Good error handling structure for symlink preparationThe new
SymlinkFixturePathsstruct andprepare_symlinksfunction provide a robust way to handle symlink creation errors, with clear documentation and proper error propagation.
117-120: Improved test robustness with early returnThe test now properly handles the case where symlinks cannot be created, returning early instead of panicking. This is a good improvement for test reliability.
161-188: Good test for unsupported DOS device pathsThe test effectively verifies that symlinks pointing to unsupported DOS device paths are treated as ordinary files/directories, which aligns with the PR objectives.
The comments provide good explanations of the expected behavior.
chenxinyanc
force-pushed
the
xinyan-dos-device-path-ut
branch
from
f7da819 to
cf894ad
Compare
chenxinyanc
force-pushed
the
xinyan-dos-device-path-ut
branch
from
cf894ad to
8bfeeae
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/tests/windows.rs (1)
44-49: Potential panic in root component extractionThe
unwrap()on the path component can panic if the path has an unexpected structure, as noted in previous reviews.let root = { // lpszVolumeMountPoint: The string must end with a trailing backslash ('\'). - let mut root = OsString::from(path.components().next().unwrap().as_os_str()); + let mut root = match path.components().next() { + Some(component) => OsString::from(component.as_os_str()), + None => return Err(Win32Error { error_code: 87 }), // ERROR_INVALID_PARAMETER + }; root.push(r"\"); root };
🧹 Nitpick comments (1)
src/tests/windows.rs (1)
41-43: Consider error handling instead of assertionThe assertion that the path has a root will cause a panic in non-test contexts. Consider returning an error instead for better robustness.
- assert!(path.has_root(), "Expected a path with a root"); + if !path.has_root() { + return Err(Win32Error { error_code: 87 }); // ERROR_INVALID_PARAMETER + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
Cargo.toml(1 hunks)README.md(1 hunks)fixtures/enhanced_resolve/test/.gitignore(1 hunks)src/tests/mod.rs(1 hunks)src/tests/symlink.rs(4 hunks)src/tests/windows.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- src/tests/mod.rs
- Cargo.toml
- fixtures/enhanced_resolve/test/.gitignore
- src/tests/symlink.rs
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/tests/windows.rs (1)
src/tests/mod.rs (1)
fixture_root(32-34)
🪛 GitHub Check: Codacy Static Code Analysis
src/tests/windows.rs
[notice] 26-26: src/tests/windows.rs#L26
Detected 'unsafe' usage, please audit for secure usage
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Test (windows-latest)
- GitHub Check: Benchmark
🔇 Additional comments (6)
src/tests/windows.rs (6)
1-11: Well-structured module with appropriate Windows configurationThe module is properly configured to compile only on Windows targets with appropriate imports for Windows-specific functionality and error handling.
11-37: LGTM: Well-documented Windows volume path conversionThe
volume_name_from_mount_pointfunction is well-documented and correctly implements the Windows API call to convert mount points to volume names. The buffer size is appropriate, and error handling returns the Windows error code.🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[notice] 26-26: src/tests/windows.rs#L26
Detected 'unsafe' usage, please audit for secure usage
25-36: Unsafe block is necessary and properly constrainedThe unsafe block is necessary for calling the Windows API and is properly constrained to just the API call. The safety comment helps document why unsafe is used here.
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[notice] 26-26: src/tests/windows.rs#L26
Detected 'unsafe' usage, please audit for secure usage
50-62: LGTM: Good DOS device path construction logicThe code correctly converts the volume name prefix and constructs the full DOS device path. The comments explaining the
\\?\to\\.\replacement are helpful.
64-69: LGTM: Well-defined error type with documentation linkThe
Win32Errorstruct is well-defined using thiserror for nice formatting and includes a link to Microsoft's error code documentation.
71-87: Comprehensive test with good validation approachThe test function correctly validates the DOS device path conversion by comparing canonicalized paths. The debug prints are helpful for troubleshooting.
|
This iteration is ready for review. |
|
|
@chenxinyanc Thanks for your amazing work! |
2 participants
This PR adds unit test for symlink targets with unsupported DOS device paths on Windows.
As mentioned in #65,
Specifically, symlinks resolving into DOS device paths with volume GUIDs won't be followed even if
symlinksoption is set totrue.This PR also updated the description of
symlinksoption in README.md.Important
Add test for symlink targets resolving to unsupported DOS device paths on Windows and update
symlinksoption documentation.symlink.rsfor symlink targets resolving to unsupported DOS device paths on Windows.windows.rsto handle DOS device paths, includingget_dos_device_path()andvolume_name_from_mount_point()functions.symlinksoption description inREADME.mdto reflect behavior with DOS device paths.windows-sys 0.59.0toCargo.tomlfor Windows-specific functionality.This description was created by
for 68d0f0d. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Documentation
Tests
New Features
Chores