refactor(rs-sdk): relocate DPNS network tests from src/ to tests/#3721
Draft
lklimek wants to merge 4 commits into
Draft
refactor(rs-sdk): relocate DPNS network tests from src/ to tests/#3721lklimek wants to merge 4 commits into
lklimek wants to merge 4 commits into
Conversation
…ORM_* vars Five DPNS network tests (one in queries.rs, four in contested_queries.rs) hardcoded https://52.12.176.90:1443 (a testnet HPMN now half-up — TCP listens, TLS hangs), which silently bypassed packages/rs-sdk/tests/.env and made vector regeneration against a local devnet or SSH tunnel impossible. Introduce a shared #[cfg(test)] helper test_address::test_address_list() that loads tests/.env (best-effort) and builds an AddressList from DASH_SDK_PLATFORM_HOST / DASH_SDK_PLATFORM_PORT / DASH_SDK_PLATFORM_SSL — matching the existing Config schema used by tests/fetch/config.rs. The five hardcoded blocks now call this helper. Network/with_network(Testnet) is left untouched.
…and-aid endpoint helper
Five #[cfg(test)] blocks in src/platform/dpns_usernames/{queries,contested_queries}.rs
were really network-integration tests — they only used pub methods on Sdk (search,
availability, resolve, contested-vote-state, etc.) and shipped their own band-aid
test_address.rs helper to read DASH_SDK_PLATFORM_* env vars. They belong in tests/,
where the existing fetch::Config harness already loads tests/.env via dotenvy/envy
and exposes address_list() against the very same env vars.
This commit:
- Relocates all five #[ignore] network tests into a new tests/dpns_usernames.rs
integration binary that re-mounts fetch::config + fetch::generated_data
(#[path] sub-modules) so it can call Config::new().address_list() instead of
parsing a hardcoded URL.
- Folds the previously hardcoded tests/dpns_queries_test.rs (which also pinned
https://52.12.176.90:1443) into the same file via the shared
build_testnet_sdk() helper, deleting the old file.
- Deletes src/platform/dpns_usernames/test_address.rs (the band-aid) and the
five #[cfg(test)] mod tests blocks. No public-API change — the production
helpers all stay where they were.
Net: -test_address.rs, -2 inline mod tests, -dpns_queries_test.rs,
+1 unified tests/dpns_usernames.rs, +consistent .env-driven endpoint config.
Run with:
cargo test -p dash-sdk --features generate-test-vectors -- --include-ignored
… local Core RPC quorum lookup build_testnet_sdk used TrustedHttpContextProvider::new(Network::Testnet, ...), which fetches quorum info from a hardcoded testnet HTTP endpoint. Three of the seven relocated DPNS tests (test_dpns_queries, test_dpns_queries_from_docs, test_dpns_search_variations) panicked with `Failed to find quorum: Quorum not found for type 106 and hash ...` when run against a local devnet or SSH tunnel — the testnet quorums simply don't match the local chain. Every other network test in `tests/fetch/*` solves this by going through `Config::setup_api(namespace)`, which wires the SDK with `SdkBuilder::with_core(host, port, user, password)`. Quorums are then resolved via the local Dash Core RPC the user is already running. This commit: - Replaces build_testnet_sdk with a one-line async helper that delegates to `Config::new().setup_api(namespace).await`. - Drops TrustedHttpContextProvider, build_testnet_context_provider, and the manual DPNS system-contract pre-load (Core RPC + standard fetch path is enough — no test depended on the pre-loaded contract for assertions). - Gives each test a unique namespace so per-test vector dump dirs don't collide when running with `--features generate-test-vectors`. The Network::Testnet hint is dropped along with the trusted context provider — `Config::setup_api` doesn't expose the network field, and the canonical `tests/fetch/*` suite operates without it just fine for proof verification via Core RPC.
…(chain-agnostic)
The relocated test asserted testnet-specific data: that "alice" was registered,
that it resolved to a hardcoded identity, that a prefix search returned at
least one row. Against a local devnet or any reset chain those assertions
fail — the chain simply doesn't have that data.
Downgrade to a call-and-log smoke test (renamed test_dpns_queries_smoke):
each SDK call still uses .expect("…should succeed") so transport or
proof-verification failures still crash loudly, but the existence of
specific on-chain rows is logged, not asserted. The test now exercises the
DPNS query code paths against any network.
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Relocates 5 network-integration tests for DPNS username helpers out of
src/platform/dpns_usernames/{queries.rs,contested_queries.rs}(where they sat as#[cfg(test)] mod testsblocks) into a newpackages/rs-sdk/tests/dpns_usernames.rs, and wires them through the existing test harness (Config::new().setup_api(namespace).awaitfromtests/fetch/config.rs).Split off from #3711 so the V0/V1 wire compatibility fix can land focused; this PR carries the pure test-organization cleanup.
What's wrong today (on v3.1-dev)
src/even though they exercise onlypub Sdkextension methods. They belong intests/.https://52.12.176.90:1443(a testnet HPMN, currently half-up — TCP listens, TLS dead). The endpoint is invisible totests/.env/Config.TrustedHttpContextProvider::new(Network::Testnet, ...)which fetches quorum info from a hardcoded testnet HTTP endpoint. Against a local devnet (e.g.,dashmate+SDK_TEST_DATA=true), quorums don't match →Failed to find quorum: Quorum not found for type 106 ….test_dpns_queries_from_docsasserts the testnet identity5DbLwAxGBzUzo81VewMUwn4b5P4bpv9FNFybi25XB5Bkexists — fails on any reset chain.tests/dpns_queries_test.rsrepeats the same hardcoded endpoint pattern; another file to maintain.What this PR does
09df598f3src/.../test_address.rsreadingDASH_SDK_PLATFORM_HOST/PORT/SSLfromtests/.envto unblock vector regen on local devnets9745211854test_address.rs+ the two#[cfg(test)] mod testsblocks insrc/.../. Deletetests/dpns_queries_test.rs. Createtests/dpns_usernames.rswith all 7 relocated tests sharing onebuild_testnet_sdk()helper. Re-mounttests/fetch/config.rs+tests/fetch/generated_data.rsvia#[path = "fetch/…"]bd7117a1f7build_testnet_sdk()throughConfig::new().setup_api(namespace).await— switches the SDK offTrustedHttpContextProvideronto the canonicalwith_core(host, port, user, password)path that resolves quorums via local Dash Core RPC75ed1d4203test_dpns_queries_from_docs→test_dpns_queries_smoke: all four existence assertions downgraded toprintln!. SDK calls still use.expect(…)so transport/proof errors panic loudly. Smoke guarantee is now "every fetch path returnedOk," not "the chain has alice."Net effect: 5 tests moved out of
src/, 1 file deleted (tests/dpns_queries_test.rs— folded in), 1 file created (tests/dpns_usernames.rs), 0 production-code changes. The methods under test were alreadypubonSdk; no surface change.The band-aid commit (
09df598f3) is preserved in history rather than squashed — it's the smallest possible step that unblocked vector regen, useful as a bisect waypoint. If you prefer a single commit, squash on merge.Test plan
cargo check -p dash-sdk --testscargo clippy -p dash-sdk --tests -- -D warningscargo fmt --alltests/.envpopulated:cargo test -p dash-sdk --features generate-test-vectors dpns_usernames -- --include-ignoredSDK_TEST_DATA=true— see Seed DPNS contested-name prerequisites via SDK_TEST_DATA (extend create_sdk_test_data) #3720 for the prerequisite gap ontest_contested_queries).Out of scope
create_sdk_test_dataextension to seed DPNS contested-name prerequisites fortests/fetch/contested_resource.rs— tracked separately in Seed DPNS contested-name prerequisites via SDK_TEST_DATA (extend create_sdk_test_data) #3720.Co-authored by Claudius the Magnificent AI Agent