Skip to content

chore: unify ckd and sign sandbox tests#1739

Merged
kevindeforth merged 7 commits intomainfrom
kd/1684-unify-sandbox-tests
Jan 13, 2026
Merged

chore: unify ckd and sign sandbox tests#1739
kevindeforth merged 7 commits intomainfrom
kd/1684-unify-sandbox-tests

Conversation

@kevindeforth
Copy link
Contributor

resolves #1684

@kevindeforth kevindeforth requested a review from gilcu3 January 11, 2026 19:43
@kevindeforth kevindeforth marked this pull request as ready for review January 11, 2026 19:49
gilcu3
gilcu3 previously approved these changes Jan 12, 2026
Copy link
Contributor

@gilcu3 gilcu3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, LGTM! Left a few comments/nits

gilcu3
gilcu3 previously approved these changes Jan 12, 2026
Copy link
Contributor

@gilcu3 gilcu3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Left a nit/fix

Co-authored-by: Reynaldo Gil Pons <gilcu3@gmail.com>
gilcu3
gilcu3 previously approved these changes Jan 12, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR successfully unifies the CKD and Sign sandbox tests by:

  • Deleting the redundant sandbox/ckd.rs file
  • Extending sign.rs to test all signature schemes including BLS12381 (CKD domain)
  • Creating a DomainResponseTest enum to handle both Sign and CKD test types
  • Refactoring helper functions to work with all domain types
  • Moving common utilities to appropriate locations

Changes:

  • Introduced DomainResponseTest enum to unify Sign and CKD test handling
  • Updated all sign tests to iterate over all signature schemes instead of just non-CKD schemes
  • Refactored test generation to use randomized inputs instead of hardcoded values
  • Moved create_account_given_id helper function from ckd.rs to common.rs for shared use

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/contract/tests/sandbox/mod.rs Removed the ckd module declaration
crates/contract/tests/sandbox/ckd.rs Deleted entire file (374 lines) - tests migrated to sign.rs
crates/contract/tests/sandbox/sign.rs Updated tests to cover all signature schemes; split single test into multiple focused tests; added CKD domain testing
crates/contract/tests/sandbox/common.rs Added create_account_given_id helper function moved from deleted ckd.rs
crates/contract/tests/sandbox/utils/sign_utils.rs Major refactoring: added DomainResponseTest enum, CKDRequestTest struct, trait ContractQueueRequest; made several functions private; removed unused imports
crates/contract/tests/sandbox/upgrade_to_current_contract.rs Updated to use new CKDResponseArgs wrapper structure

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pbeza
Copy link
Contributor

pbeza commented Jan 12, 2026

@kevindeforth Sorry, I got caught up fixing my PRs. I’ll review it early tomorrow morning.

@kevindeforth kevindeforth force-pushed the kd/1684-unify-sandbox-tests branch from 8d75918 to df27d57 Compare January 13, 2026 08:31
pbeza
pbeza previously approved these changes Jan 13, 2026
@kevindeforth kevindeforth added this pull request to the merge queue Jan 13, 2026
Merged via the queue into main with commit 8abdba9 Jan 13, 2026
15 checks passed
@kevindeforth kevindeforth deleted the kd/1684-unify-sandbox-tests branch January 13, 2026 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

unify sandbox/ckd.rs and sandbox/sign.rs

4 participants