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

@zackees zackees 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

coderabbitai Bot 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 main Jul 25, 2026
92 of 93 checks passed
@zackees
zackees deleted the fix/issue-1155-dylint-tests branch July 25, 2026 22:40
@fastled-project-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Jul 26, 2026
Sign up for free to 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