Skip to content

Conversation

@kevindeforth
Copy link
Contributor

@kevindeforth kevindeforth commented Dec 16, 2025

resolves #1666

main improvements:

  • all except one test inside sign.rs no longer rely sleeps (edit: almost, there is still one sleep we can't get rid of with the current sandbox capabilities, c.f. slack)
  • tests inside sing.rs are significantly faster, due to the use of "fast forward"

follow-ups:

  1. chore(contract): sandbox code organization #1683
  2. unify sandbox/ckd.rs and sandbox/sign.rs #1684
  3. a re-scoped don't sleep in integration tests #1306

@kevindeforth kevindeforth force-pushed the kd/1666-fix-sandbox-sign-test branch from 7f4699c to 7a8fc95 Compare December 17, 2025 12:09
@kevindeforth kevindeforth force-pushed the kd/1666-fix-sandbox-sign-test branch from 7a8fc95 to 5e349ea Compare December 17, 2025 12:11
@kevindeforth kevindeforth marked this pull request as ready for review December 17, 2025 12:37
@kevindeforth kevindeforth linked an issue Dec 17, 2025 that may be closed by this pull request
gilcu3
gilcu3 previously approved these changes Dec 17, 2025
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 only minor fixes and a couple of questions

pbeza
pbeza previously approved these changes Dec 17, 2025
@kevindeforth kevindeforth dismissed stale reviews from pbeza and gilcu3 via 7b11c04 December 17, 2025 15:50
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 again for the effort, I guess #1306 will need to remain unsolved for now

@kevindeforth
Copy link
Contributor Author

Thank you again for the effort, I guess #1306 will need to remain unsolved for now

good point. I added some info to that issue

@kevindeforth kevindeforth added this pull request to the merge queue Dec 17, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 17, 2025
Copy link
Collaborator

@netrome netrome left a comment

Choose a reason for hiding this comment

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

Quick skimming, thanks for doing this!

.await
.unwrap();
let status_2 = req.sign_ensure_included(&alice, &contract).await?;
// unfortunately, we still can't completely get rid of this sleep
Copy link
Collaborator

Choose a reason for hiding this comment

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

😢

@netrome netrome added this pull request to the merge queue Dec 17, 2025
@netrome
Copy link
Collaborator

netrome commented Dec 17, 2025

Setting this to merge as @kevindeforth is on vacation tomorrow.

Merged via the queue into main with commit 4b863f4 Dec 17, 2025
17 checks passed
@netrome netrome deleted the kd/1666-fix-sandbox-sign-test branch December 17, 2025 20:59
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.

test: fix sandbox "sign"

5 participants