ci: cover rs-unified-sdk-jni and missing contract crates in package filters#4203
Conversation
…ilters Pushes and PRs touching only packages/rs-unified-sdk-jni never triggered the Rust workspace tests job, because the crate was absent from rs-packages-no-workflows.yml — so cargo fmt/clippy/test were skipped entirely for it. That is how misformatted code landed on v4.1-dev and broke the formatting check for every unrelated PR (fixed directly in bcd0ac2). The dedicated kotlin-sdk-build workflow does trigger on the crate but only builds and runs emulator tests; it never runs fmt or clippy. Audited every .github/package-filters file against the Cargo workspace members and yarn workspaces: - rs filters: add rs-unified-sdk-jni (with its platform-wallet-ffi and rs-sdk-ffi dependency closure) and document-history-contract (including the data-contracts alias, mirroring its optional dep in packages/data-contracts). - js filters: add @dashevo/document-history-contract and @dashevo/keyword-search-contract — both are yarn workspaces with mocha unit suites that were never wired into the JS test matrix — and alias them into @dashevo/wasm-dpp like the other embedded contracts. - tests.yml: add document-history-contract to the inline swift-sdk-changed filter (rs-sdk-ffi embeds it via data-contracts). - test-suite-triggers.yml: add both contracts to the system-contracts section for consistency. All 46 workspace members are now covered by the live Rust filter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
🕓 Ready for review — 59 ahead in queue (commit 639964f) |
Three more v4.1-dev breakages that only surface on full (dispatch or version-change) CI runs: - Docker image builds fail at cargo-chef prepare: #4171 made document-history-contract a dependency of data-contracts but the Dockerfile never copies the new crate. Add it everywhere token-history-contract appears. This also broke Test Suite and Dashmate E2E, which need the images. - keyword-search-contract unit tests were copy-pasted from wallet-utils-contract and still tested its txMetadata document type, which this contract does not define; every case threw. Rewrite the suite against the real schema (contractKeywords, shortDescription, fullDescription). Two behaviors surfaced while doing so are asserted as such: wrong-length identifier fields throw at document creation (identifier conversion precedes schema validation), and fullDescription's schema maxLength (10000) sits above the system per-field cap (5120), so overlength strings yield a field-size error, not a maxLength error. - document-history-contract asserted a minimum on purchase.price, but the schema deliberately leaves the integer unbounded so the property stays sum-aggregatable; consensus validates actual amounts. Assert the type check and the intentional absence of a schema minimum instead. These suites were never exercised in CI before #4203 added the missing package filters, and the contract-test jobs only run on full pipelines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 participants
Issue being fixed or feature implemented
Pushes and PRs that touch only
packages/rs-unified-sdk-jninever trigger the Rust workspace tests job: the crate is absent from.github/package-filters/rs-packages-no-workflows.yml, so thechangesjob intests.ymlreports no Rust packages changed andcargo fmt/clippy/cargo testare skipped entirely. This is exactly how misformatted code from #4192 landed onv4.1-devand broke thecargo fmt --check --allstep for every unrelated PR (fixed directly in bcd0ac2). The dedicatedkotlin-sdk-build.ymlworkflow does trigger on the crate, but it only builds the JNI library and runs emulator tests — it never runs fmt or clippy.Auditing every file in
.github/package-filters/against the rootCargo.tomlworkspace members and the yarn workspaces turned up more of the same class of gap:packages/document-history-contract(Rust crate, added with the document-history work) had no filter entry and was missing from thedata-contractsalias — contract-only changes skipped Rust CI too.@dashevo/document-history-contractand@dashevo/keyword-search-contractare yarn workspaces with mocha unit suites that were never wired into the JS test matrix.document-history-contractwas also missing from the inlineswift-sdk-changedfilter intests.yml, even thoughrs-sdk-ffiembeds it viadata-contracts.What was done?
rs-packages-no-workflows.yml(gates the Rust workspace tests): addedrs-unified-sdk-jniwith its dependency closure (platform-wallet-ffi+rs-sdk-ffialiases, mirroring thers-unified-sdk-ffientry), anddocument-history-contract(own entry +data-contractsalias, mirroring its optional dependency inpackages/data-contracts/Cargo.toml).js-packages-no-workflows.yml/js-packages-direct.yml(gate the JS test matrix): added entries for both contract packages and aliased them into@dashevo/wasm-dpplike the other embedded system contracts. Neither has JS-side dependents, so no other consumer lists change.tests.yml: addedpackages/document-history-contract/**to the inlineswift-sdk-changedfilter.rs-packages.yml,rs-packages-direct.yml,js-packages.yml,test-suite-triggers.yml: same additions for consistency. Note these four files are not referenced by any workflow anymore (only the-no-workflowspair andjs-packages-direct.ymlare used bytests.yml), so they are kept in sync here rather than extended in behavior — deleting them could be a follow-up if they are truly dead.After this change, all 46 Cargo workspace members are matched by at least one pattern in the live Rust filter.
How Has This Been Tested?
Validated with a script that:
Cargo.tomlagainst the live Rust filter's patterns — no uncovered members remain;dorny/paths-filtermatching: apackages/rs-unified-sdk-jni/src/lib.rschange now matches thers-unified-sdk-jnikey (sors-packages != '[]'and the Rust workspace job runs), and adocument-history-contractchange cascades throughdata-contracts→dpp→ dependents, matching the behavior of the existing contract crates.Breaking Changes
None — CI-only. The new filter keys can only add test jobs for changes that previously ran none.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code