Skip to content

test(usb): codify #740 vendor-VID resolution table as a unit test#922

Merged
zackees merged 1 commit into
mainfrom
fix/740-vendor-vid-table-test
Jul 1, 2026
Merged

test(usb): codify #740 vendor-VID resolution table as a unit test#922
zackees merged 1 commit into
mainfrom
fix/740-vendor-vid-table-test

Conversation

@zackees

@zackees zackees commented Jul 1, 2026

Copy link
Copy Markdown
Member

Closes #740.

Summary

The Vendor-resolution results table in #740 has been the manual verification pass every online-data publish: 19 headline VIDs, each verified via curl .../usb-vid.json | jq '.["VVVV"].vendor'. Every prior run has been a gh + jq sweep by hand — and #740 was pinned open as the "current state of the world" only because there was no programmatic guard against drift.

Freezes the entire 19-row table into usb::embedded::tests::embedded_resolves_every_issue_740_vendor so CI catches any archive regression at build time.

Also fixes the same drift-tracking gap for the Board-name match results table by extending the sister test at crates/fbuild-config/src/board/tests_common_board_vidpid.rs — that one was already added by #920 (commit 3f4c3ffe), so this PR only adds the vendor half.

Three drift rows called out in the test

Three VIDs in the #740 table are vendor-override targets of online-data-tools/vendor_names_inlined.py, whose overlay is applied to usb-vid.json via overlay_usb_vid.py --mode vendor-override:

  • 0x045B — upstream usb.ids says "Hitachi, Ltd"; overlay renames to "Renesas Electronics".
  • 0x2A03 — upstream says "dog hunter AG" (the VID holder); overlay renames to "Arduino LLC" (the downstream licensee).
  • 0x2544 — missing from upstream entirely; overlay adds as "Silicon Labs".

The test accepts BOTH the upstream and overlay label for 0x045B / 0x2A03 so it survives an archive rebuild in either direction. 0x2544 is treated as a soft row — a missing entry is reported (eprintln!) but not fatal, since the archive rebuild is what pulls it in.

Test plan

  • soldr --no-cache cargo test -p fbuild-core --lib usb::embedded — exit 0 (5 tests pass including the new embedded_resolves_every_issue_740_vendor).
  • soldr --no-cache cargo clippy -p fbuild-core --lib -- -D warnings — exit 0.
  • soldr cargo fmt -p fbuild-core -- --check — exit 0.

Follow-up: closing #740

With this test in place plus the sister board-name test from #920, both #740 tables are now enforced by CI on every commit. #740 stays open only as a live-pin per its body text — but the underlying "drift is silently possible" concern is now closed. Recommend closing #740 with a comment linking here + #920.

🤖 Generated with Claude Code

Closes #740.

The `Vendor-resolution results` table in #740 has been the manual
verification pass every `online-data` publish: 19 headline VIDs, each
verified via `curl .../usb-vid.json | jq '.["VVVV"].vendor'`. Every
prior run has been a `gh` + `jq` sweep by hand, and #740 was pinned
open as the "current state of the world" only because there was no
programmatic guard against drift.

Freeze the entire 19-row table into `usb::embedded::tests::
embedded_resolves_every_issue_740_vendor` so CI catches any archive
regression at build time. Rows match the issue body's table verbatim
(same VIDs, same expected-vendor substrings). For each row the test
looks up the embedded archive's vendor name and confirms a
case-insensitive substring match against ANY of the accepted
alternatives, so upstream-label variance (e.g. "Silicon Labs" vs
"Cygnal") never causes spurious churn.

## Three drift rows called out in the test

Three VIDs in the #740 table are `vendor-override` targets of
`online-data-tools/vendor_names_inlined.py`, whose overlay is applied
to `usb-vid.json` via `overlay_usb_vid.py --mode vendor-override`:

  * 0x045B — upstream `usb.ids` says "Hitachi, Ltd"; overlay renames
    to "Renesas Electronics".
  * 0x2A03 — upstream says "dog hunter AG" (the VID holder); overlay
    renames to "Arduino LLC" (the downstream licensee).
  * 0x2544 — missing from upstream entirely; overlay adds as
    "Silicon Labs".

The test accepts BOTH the upstream and overlay label for 0x045B /
0x2A03 so it survives an archive rebuild in either direction. 0x2544
is treated as a soft row — a missing entry is reported (`eprintln!`)
but not fatal, since the archive rebuild is what pulls it in.

## Verified locally

- `soldr --no-cache cargo test -p fbuild-core --lib usb::embedded` — exit 0
  (5 tests pass including the new `embedded_resolves_every_issue_740_vendor`).
- `soldr --no-cache cargo clippy -p fbuild-core --lib -- -D warnings` — exit 0.
- `soldr cargo fmt -p fbuild-core -- --check` — exit 0.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@zackees, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 12 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ed412194-0f4a-4644-96bd-961a255af4d8

📥 Commits

Reviewing files that changed from the base of the PR and between 8a45859 and df7bd17.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • crates/fbuild-core/src/usb/embedded.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/740-vendor-vid-table-test

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 merged commit 739bc31 into main Jul 1, 2026
89 of 93 checks passed
zackees added a commit that referenced this pull request Jul 1, 2026
#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-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Jul 2, 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.

vid-ingest: final end-to-end verification (after #723–#739 close)

1 participant