Skip to content

fix(dylint): normalize remaining test paths - #1194

Merged
zackees merged 1 commit into
mainfrom
fix/issue-1155-dylint-tests
Jul 25, 2026
Merged

fix(dylint): normalize remaining test paths#1194
zackees merged 1 commit into
mainfrom
fix/issue-1155-dylint-tests

Conversation

@zackees

@zackeeszackees commented Jul 25, 2026

Copy link
Copy Markdown
Member

Summary

  • replace remaining raw PathBuf constructions in daemon ownership and board VID/PID tests with NormalizedPath
  • route the cross-process ownership probe through fbuild_core::subprocess::run_command_blocking with a 30-second bound
  • resolve the next workspace-wide Dylint 6.0.1 findings exposed after fix(dylint): normalize remaining workspace paths #1193 merged early

Validation

  • soldr cargo test -p fbuild-paths root_ownership_is_version_blind_across_processes
  • soldr cargo test -p fbuild-config bundled_board_snapshots_never_embed_usb_vid_or_pid
  • soldr cargo fmt --all -- --check
  • no lint or allowlist changes

Follow-up to #1155, #1192, and #1193.

Summary by CodeRabbit

  • Tests
    • Improved validation of bundled board paths using consistent normalized path handling.
    • Expanded coverage for ownership claims, subprocess behavior, and cross-process lock handling.
    • Added more reliable command execution checks with explicit environment and timeout controls.

Route the daemon ownership subprocess probe through fbuild's captured runner and use NormalizedPath in both affected test modules.
@coderabbitai

coderabbitaiBot commented Jul 25, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 01a50da6-d850-41d9-a593-652f59d9ee39

📥 Commits

Reviewing files that changed from the base of the PR and between becc05e and 5db6352.

📒 Files selected for processing (2)
  • crates/fbuild-config/src/board/tests_common_board_vidpid.rs
  • crates/fbuild-paths/src/daemon_ownership.rs

📝 Walkthrough

Walkthrough

Test helpers and daemon ownership tests now consistently use NormalizedPath; subprocess probing uses the shared blocking command runner with explicit arguments, environment, and timeout configuration.

Changes

Path normalization in tests

Layer / File(s)Summary
Board directory path helper
crates/fbuild-config/src/board/tests_common_board_vidpid.rs
bundled_boards_dir now returns NormalizedPath and constructs the bundled board path through NormalizedPath::new(...).join(...).
Daemon ownership test paths and subprocess handling
crates/fbuild-paths/src/daemon_ownership.rs
Ownership test fixtures and lock paths now use NormalizedPath; subprocess probing uses run_command_blocking with explicit arguments, environment, and a 30-second timeout.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-1155-dylint-tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zackees
zackees marked this pull request as draft July 25, 2026 22:23
@zackees
zackees marked this pull request as ready for review July 25, 2026 22:40
@zackees
zackees merged commit fad23bb into mainJul 25, 2026
92 of 93 checks passed
@zackees
zackees deleted the fix/issue-1155-dylint-tests branch July 25, 2026 22:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant

@zackees