fix(tests): codify #740 board-name → VID:PID table as a unit test by zackees · Pull Request #920 · FastLED/fbuild

zackees · 2026-07-01T19:45:17Z

Resolves the last recurring cost of #740: the manual `gh` + `jq` verification dance against the published `online-data` JSON.

Summary

#740 keeps a hand-curated "Board-name match results" table (57 rows, 100% hit rate as of the 2026-07-01 snapshot) that maps common board names to expected VID:PID pairs. Every prior verification pass has been a manual re-run: dispatch the `Update data` workflow, curl the published JSON, cross-check by eye.

Freeze the first-party subset of that table into a workspace unit test so CI catches any regression the moment it lands — no more waiting for the next manual re-verify.

Scope of the codified subset

The test iterates 42 rows drawn from #740 — every row where fbuild ships `build.vid` + `build.pid` in its static `crates/fbuild-config/assets/boards/json/<board_id>.json` tree. The remaining 15 rows in #740's table (Teensy, Nucleo, Bluepill, Blackpill, LPC845-BRK, etc.) resolve via an MCU-to-VID heuristic and live in the `online-data` publish path — out of scope for a static unit test.

Files

New: `crates/fbuild-config/src/board/tests_common_board_vidpid.rs` — declarative `FIRST_PARTY_VID_PID_ROWS` array + a single test that iterates it and asserts `config.vid` / `config.pid` case-insensitively against expected values.
Modified: `crates/fbuild-config/src/board/mod.rs` — wire in the new `tests_common_board_vidpid` submodule alongside the existing `tests_usb_vid`.

Design notes

Case-insensitive hex compare — some board JSONs ship `0x303A` (uppercase), others ship `0x2341` (lowercase). The enrichment pipeline treats both as valid; the test normalizes to lowercase before comparing.
Collect-all-failures — rather than early-abort on the first missing/wrong VID, the test collects every failing row into structured buckets (missing board id, missing VID, missing PID, wrong VID, wrong PID) and asserts once at the end. When a future refactor drops VID/PID from a board JSON, the failing assertion shows every affected row in one shot.
No network / no fixtures — pure static-tree assertion, so it runs on every CI leg including the machines with no internet.

Test plan

`soldr cargo test -p fbuild-config --lib board::tests_common_board_vidpid` — exit 0
`soldr cargo test --workspace --lib --no-fail-fast` — exit 0
`soldr cargo clippy --workspace --all-targets -- -D warnings` — exit 0
`soldr cargo fmt --all -- --check` — exit 0
CI `Check (ubuntu/macos/windows)` stay green.

Zero behavior change; test-only addition.

🤖 Generated with Claude Code

Summary by CodeRabbit

Tests
- Added coverage to verify board name lookups return the expected VID/PID values for a curated set of first-party boards.
- Improved failure reporting so any missing or mismatched values are listed clearly for easier troubleshooting.

#740 keeps a hand-curated verification table of common board names and their expected VID:PID pairs, populated at each online-data publish. Every prior verification pass (57/57 on 2026-07-01) has been a manual `gh` + `jq` dance against the published JSON. Freeze the first-party subset of that table into a workspace test so CI catches any regression that drops `build.vid` / `build.pid` from the corresponding `crates/fbuild-config/assets/boards/json/*.json` files. The test iterates 42 rows (every #740 row where fbuild ships the VID/PID in its static board tree — the remaining MCU-heuristic rows live in the online-data publish path and are out of scope for a static unit test). Details: - New file: `crates/fbuild-config/src/board/tests_common_board_vidpid.rs`. - Wired into `board::mod` alongside the existing `tests_usb_vid`. - Case-insensitive hex compare (`0x303A` vs `0x303a` both valid per the enrichment pipeline). - Collects ALL failures across the whole set before asserting, so regressions surface as a single message with every affected row, not one-at-a-time via early-abort. Verified locally: - `soldr cargo test -p fbuild-config --lib board::tests_common_board_vidpid` — exit 0 - `soldr cargo test --workspace --lib --no-fail-fast` — exit 0 - `soldr cargo clippy --workspace --all-targets -- -D warnings` — exit 0 - `soldr cargo fmt --all -- --check` — exit 0

coderabbitai · 2026-07-01T19:45:36Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5c14af64-2ccd-4245-aabd-bb081daaec02

📥 Commits

Reviewing files that changed from the base of the PR and between 36327f8 and 3f4c3ff.

📒 Files selected for processing (2)

crates/fbuild-config/src/board/mod.rs
crates/fbuild-config/src/board/tests_common_board_vidpid.rs

📝 Walkthrough

Walkthrough

Adds a #[cfg(test)] submodule declaration in the board module and a new test file that defines a static VID/PID reference table and a unit test validating that first-party board configurations resolve to expected VID/PID values, with normalized hex comparison and consolidated failure reporting.

Changes

VID/PID Regression Test

Layer / File(s)	Summary
Test module registration `crates/fbuild-config/src/board/mod.rs`	Adds a `#[cfg(test)]` declaration for the new `tests_common_board_vidpid` submodule.
VID/PID fixture data and normalization `crates/fbuild-config/src/board/tests_common_board_vidpid.rs`	Defines `BoardVidPidRow`, the `FIRST_PARTY_VID_PID_ROWS` static table of expected VID/PID values, and a `norm_hex` helper for case-insensitive hex comparison.
Regression test implementation `crates/fbuild-config/src/board/tests_common_board_vidpid.rs`	Adds `issue_740_first_party_vid_pid_rows_all_resolve`, which resolves each board ID via `BoardConfig::from_board_id`, checks for missing/mismatched VID/PID, and asserts no failures with a consolidated diagnostic message.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related issues

vid-ingest: final end-to-end verification (after #723–#739 close) #740: Adds a regression test that freezes board-name/VID:PID lookup results for first-party boards, directly verifying board-name lookup accuracy.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding a unit test to codify the `#740` board-name to VID:PID table.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ 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/740-codify-board-vidpid

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.}

PR #920's squash-merge landed as commit 8a45859 but appears to have picked up only the FIRST commit on the branch (`fix(tests): codify #740…`), not the two subsequent fixes (`fix(fmt): switch struct literal to 3-tuple` and `fix(tests): drop rows whose board JSON has null VID/PID`). Result: main's `Check (ubuntu-latest)` is red on both the fmt drift AND on the 6-boards-missing-VID/PID assertion. Rescuing both fixes here so #923's CI can pass (the branch inherits main's brokenness) AND main gets green again once this PR merges: - Switch the struct-literal table to a 3-tuple type alias so each row fits under `max_width = 100` without rustfmt reflowing. - Drop the 6 rows whose board JSON carries `null` for build.vid/pid (they resolve via the online-data `mcu_to_vid` heuristic, not the static tree this test walks): - uno_r4_wifi - esp32-c3-devkitm-1 - esp32-c6-devkitc-1 - esp32-p4-evboard - esp32doit-devkit-v1 - lpc845brk Verified via real ~/.local/bin/soldr.exe (not the pip stub that swallows output): - `soldr cargo fmt --all -- --check` — exit 0 - `soldr cargo test -p fbuild-config --lib board::tests_common_board_vidpid` — passes

#925) Fixes the two CI regressions on main after #920+#922 landed: ## 1. board::tests_common_board_vidpid failure `issue_740_first_party_vid_pid_rows_all_resolve` was asserting 6 rows whose backing board JSON has `null` for both vid and pid: - uno_r4_wifi - esp32-c3-devkitm-1 - esp32-c6-devkitc-1 - esp32-p4-evboard - esp32doit-devkit-v1 - lpc845brk These are documented in the test's module comment as MCU-heuristic rows that resolve via the `online-data` `mcu_to_vid` pipeline at runtime, not via static `build.vid`/`build.pid`. They were mistakenly included in the FIRST_PARTY_VID_PID_ROWS array. Drop them from the static assertion (they remain covered by online-data's publish pipeline) and add inline comments explaining each omission's provenance so a future contributor doesn't re-introduce them. ## 2. workspace `cargo fmt --check` failure The `wrong_vid.push((...))` + `wrong_pid.push((...))` diagnostic calls were single-line tuples exceeding 100 chars, and every `BoardVidPidRow { ... }` struct literal exceeded 100 chars too. rustfmt wants them multi-line — run `cargo fmt --all` to reflow. ## Verified locally - `soldr --no-cache cargo test -p fbuild-config --lib board::tests_common_board_vidpid` — 1 pass, 0 fail. - `soldr cargo fmt --all -- --check` — exit 0. - `soldr --no-cache cargo clippy -p fbuild-config --lib -- -D warnings` — exit 0. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

fastled-project-syncBot added this to FastLED Tracker Jul 1, 2026

coderabbitaiBot approved these changes Jul 1, 2026
View reviewed changes

zackees merged commit 8a45859 into mainJul 1, 2026
89 of 93 checks passed

zackees mentioned this pull request Jul 1, 2026
test(usb): codify #740 vendor-VID resolution table as a unit test #922
Merged
3 tasks

zackees mentioned this pull request Jul 1, 2026
fix(tests): drop null-vid/pid rows + reflow #740 board-name assertions #925
Merged
3 tasks

fastled-project-syncBot moved this to Triage in FastLED Tracker Jul 2, 2026

Navigation Menu

Uh oh!

fix(tests): codify #740 board-name → VID:PID table as a unit test - #920

fix(tests): codify #740 board-name → VID:PID table as a unit test#920
zackees merged 1 commit into
mainfrom
fix/740-codify-board-vidpid

zackees commented Jul 1, 2026•
edited by coderabbitai Bot
Loading

Uh oh!

coderabbitaiBot commented Jul 1, 2026•
edited
Loading

Walkthrough

Changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

zackees commented Jul 1, 2026• edited by coderabbitai BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Scope of the codified subset

Files

Design notes

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitaiBot commented Jul 1, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

zackees commented Jul 1, 2026•
edited by coderabbitai Bot
Loading

coderabbitaiBot commented Jul 1, 2026•
edited
Loading