Cleanup and test rework #17116
Merged
Cleanup and test rework #17116
Conversation
This file contains 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
dariorussi
requested review from
patrickkuo and
longbowlu
and removed request for
ronny-mysten
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Ignored Deployments
|
|
this can go in as is or wait for me to add more tests to |
longbowlu
reviewed
Apr 15, 2024
Comment on lines
+720
to
+753
| #[test_only] | ||
| public fun transfer_status_pending(): u8 { | ||
| TRANSFER_STATUS_PENDING | ||
| } |
There was a problem hiding this comment.
can we make these const public instead?
There was a problem hiding this comment.
unfortunately there is no public const concept in move, all const are private to the module
longbowlu
reviewed
Apr 15, 2024
Comment on lines
+608
to
+632
| #[test_only] | ||
| public fun setup_treasury_for_testing(bridge: &mut Bridge) { | ||
| bridge.load_inner_mut().treasury.setup_for_testing(); | ||
| } |
There was a problem hiding this comment.
for these kind of test-only helpers, is it better to just make them public(package or friend (if it's still a thing in move 2024?) @dariorussi
There was a problem hiding this comment.
it does not really matter because those are compiled away.
I am not sure if there is a "standard" here, I'll find out.
We are talking about unit tests having visibility over the module they are testing but that is a change on the compiler side (and maybe more) that is not scheduled or planned yet
There was a problem hiding this comment.
cool, i'm curious too but not blocking this PR
longbowlu
reviewed
Apr 15, 2024
| abort TEST_DONE | ||
| } | ||
|
|
||
| // #[test] |
There was a problem hiding this comment.
why do we have these commented?
There was a problem hiding this comment.
because they do not work yet as I need to build more infra for those tests... I did not want to have them completely out yet, which I could if we are going to get this in as it is
longbowlu
approved these changes
Apr 16, 2024
patrickkuo
pushed a commit
that referenced
this pull request
May 2, 2024
## Description More Move2024 cleanup and formatting. Moved tests out of `bridge.move` in its own test directory. We will transition all tests to that model as we work towards 100% test coverage. ## Test plan This is mostly tests --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:
patrickkuo
pushed a commit
that referenced
this pull request
May 8, 2024
## Description More Move2024 cleanup and formatting. Moved tests out of `bridge.move` in its own test directory. We will transition all tests to that model as we work towards 100% test coverage. ## Test plan This is mostly tests --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:
patrickkuo
pushed a commit
that referenced
this pull request
May 9, 2024
## Description More Move2024 cleanup and formatting. Moved tests out of `bridge.move` in its own test directory. We will transition all tests to that model as we work towards 100% test coverage. ## Test plan This is mostly tests --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:
patrickkuo
pushed a commit
that referenced
this pull request
May 9, 2024
## Description More Move2024 cleanup and formatting. Moved tests out of `bridge.move` in its own test directory. We will transition all tests to that model as we work towards 100% test coverage. ## Test plan This is mostly tests --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:
patrickkuo
pushed a commit
that referenced
this pull request
May 9, 2024
## Description More Move2024 cleanup and formatting. Moved tests out of `bridge.move` in its own test directory. We will transition all tests to that model as we work towards 100% test coverage. ## Test plan This is mostly tests --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:
patrickkuo
pushed a commit
that referenced
this pull request
May 9, 2024
## Description More Move2024 cleanup and formatting. Moved tests out of `bridge.move` in its own test directory. We will transition all tests to that model as we work towards 100% test coverage. ## Test plan This is mostly tests --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:
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.
Description
More Move2024 cleanup and formatting.
Moved tests out of
bridge.movein its own test directory.We will transition all tests to that model as we work towards 100% test coverage.
Test plan
This is mostly tests
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.