Restore Node/Python binding tests for BReg apply reasons

Open
#1,216 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
76/100
Issue type
Refactor
Clarity
Clearly specified
Activity status
Active
Tech stack
node.js, python, rust
Domain
backend, testing-qa

Research direction

Start with crates/registry-breg-client/tests/fixtures/review-reasons.json and the deleted-path references in crates/registry-breg-client-node/test/review-reasons.test.js and crates/registry-breg-client-py/tests/python/test_review_reasons.py. Read the Rust contract at crates/registry-breg-client/tests/lifecycle_contract.rs:75-83, then run the Node and Python binding test suites. Done means both bindings cover the bounded with_reason contract, or the suites explicitly document why Rust-only coverage is sufficient.

Written by the indexing model from the issue text.

Description

area:breg bug good first issue

The Node.js and Python binding tests for BReg's apply-reason API were deleted without replacement. As of PR #1189 (commit 47000edc5), crates/registry-breg-client-node/__test__/review-reasons.test.js, crates/registry-breg-client-py/tests/python/test_review_reasons.py, and the shared fixture crates/registry-breg-client/tests/fixtures/review-reasons.json were removed. The with_reason API itself is still exercised at the Rust level in crates/registry-breg-client/tests/lifecycle_contract.rs:75-83 (only_apply_accepts_a_bounded_reason), but the two language bindings that wrap it have no equivalent coverage anymore.

Failure scenario: a future change to the Node or Python binding layer for with_reason (bounds checking, Unicode handling, the Debug redaction of the reason text) regresses silently, since only the Rust-level test would catch a logic bug, and the bindings' own serialization/FFI boundary isn't exercised for this API at all.

Suggested fix:

  • Restore an equivalent Node and Python test (they don't need to be identical to the deleted ones, just re-cover the bounded reason contract at the binding layer) or explicitly note in the binding test suites why Rust-level coverage is considered sufficient.

Filed from the review of #1189; not merge-blocking.

Dominant language
Rust
Stars
2
Forks
0
Avg merge
3h 45m
Merged PRs (30d)
134

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from registrystack/registry-stack

All issues in registrystack/registry-stack

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.