Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[move-compiler] public init for unit testing in debug build #20481

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

yanganto
Copy link
Contributor

@yanganto yanganto commented Dec 2, 2024

Description

Provide init for unit test in debug build, please reference #20389.

Test plan

cross_module_init_function.move is added for make sure there is no change on the release baseline.


Release notes

The release did no change any production level code.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • Indexer:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:
  • REST API:

Copy link

vercel bot commented Dec 2, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 14, 2024 1:12pm
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Dec 14, 2024 1:12pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Dec 14, 2024 1:12pm
sui-typescript-docs ⬜️ Ignored (Inspect) Visit Preview Dec 14, 2024 1:12pm

@stefan-mysten stefan-mysten requested review from tnowacki and tzakian and removed request for tnowacki December 2, 2024 17:39
@stefan-mysten stefan-mysten changed the title [move-compiler] public init for unittesting in dubug build [move-compiler] public init for unit testing in debug build Dec 2, 2024
Copy link
Contributor

@tnowacki tnowacki left a comment

Choose a reason for hiding this comment

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

Appreciate the effort on this! And sorry for not getting back to this sooner.
As the comment suggests, we have an issue here where the Sui bytecode verifier needs special support for this on the unit testing side. You won't be able to see this in the tests just in external-crates, and it will be visible only on the Sui side.
This isn't something that can be solved with Rust's debug mode (#[cfg(debug_assertions)]), since we release and build the CLI in release mode

@yanganto
Copy link
Contributor Author

yanganto commented Dec 3, 2024

Appreciate the effort on this! And sorry for not getting back to this sooner. As the comment suggests, we have an issue here where the Sui bytecode verifier needs special support for this on the unit testing side. You won't be able to see this in the tests just in external-crates, and it will be visible only on the Sui side. This isn't something that can be solved with Rust's debug mode (#[cfg(debug_assertions)]), since we release and build the CLI in release mode

I try to do this in debug build, because the test coverage only work in debug build now.
So I always use debug build with nearly latest commit in Github action to do test and also calculate test coverage.
If this is also good in release build for now, the #[cfg(debug_assertions)] will be removed. 😄

@yanganto
Copy link
Contributor Author

Now there is no debug_assertions in the PR, and it will effect in release build with sui-mode and the test now in sui-mode.

@yanganto
Copy link
Contributor Author

Hi @tnowacki,
The build from the PR now works well, the visibility for init is working well in test scenario now. If there anything I need to change also in this PR, please let me know and I am happy to do it. 🙏

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.

2 participants