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.

Sign up for GitHub

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

Jump to bottom

test view function with struct arguments and generic type parameters #7761

Merged
merged 7 commits into from
Apr 14, 2023
Merged

test view function with struct arguments and generic type parameters #7761

merged 7 commits into from
Apr 14, 2023

Conversation

gerben-stavenga
Copy link
Contributor

Description

Add test for a view function with a generic type parameter taking Object

Test Plan

@gerben-stavenga gerben-stavenga changed the title Gerben test view test view function with struct arguments and generic type parameters Apr 14, 2023
@gerben-stavenga gerben-stavenga requested review from davidiw, lightmark and movekevin and removed request for banool April 14, 2023 02:50
movekevin
movekevin approved these changes Apr 14, 2023
View reviewed changes
Copy link
Contributor

@movekevin movekevin left a comment

Choose a reason for hiding this comment

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

Nice! Cc @xindingw

@movekevin movekevin requested a review from xindingw April 14, 2023 03:41
movekevin
movekevin reviewed Apr 14, 2023
View reviewed changes
Copy link
Contributor

@movekevin movekevin left a comment

Choose a reason for hiding this comment

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

Why not add an API test instead to make sure things work e2e?

xindingw
xindingw reviewed Apr 14, 2023
View reviewed changes
aptos-move/e2e-move-tests/src/tests/constructor_args.rs
@@ -21,21 +25,26 @@ const OBJECT_ADDRESS: AccountAddress = AccountAddress::new([
0x90, 0x71, 0xAA, 0x3F, 0xBD, 0x2A, 0xB9, 0x51, 0x37, 0xF7, 0xCB, 0xAD, 0x13, 0x6F, 0x09, 0x2B,
]);

fn success(tests: Vec<(&str, Vec<Vec<u8>>, &str)>) {
success_generic(vec![], tests)
fn module_data() -> StructTag {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe have a static variable with lazy initialization like here

@gerben-stavenga gerben-stavenga enabled auto-merge (squash) April 14, 2023 17:39
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

✅ Forge suite land_blocking success on e0a121688371bf72e989daeac2fffda5447ae01d

performance benchmark with full nodes : 5655 TPS, 7010 ms latency, 11100 ms p99 latency,no expired txns
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> e0a121688371bf72e989daeac2fffda5447ae01d

Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> e0a121688371bf72e989daeac2fffda5447ae01d (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 8133 TPS, 4692 ms latency, 6900 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: e0a121688371bf72e989daeac2fffda5447ae01d
compatibility::simple-validator-upgrade::single-validator-upgrade : 4352 TPS, 9194 ms latency, 12000 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: e0a121688371bf72e989daeac2fffda5447ae01d
compatibility::simple-validator-upgrade::half-validator-upgrade : 4747 TPS, 8525 ms latency, 11700 ms p99 latency,no expired txns
4. upgrading second batch to new version: e0a121688371bf72e989daeac2fffda5447ae01d
compatibility::simple-validator-upgrade::rest-validator-upgrade : 6754 TPS, 5757 ms latency, 9100 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> e0a121688371bf72e989daeac2fffda5447ae01d passed
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite framework_upgrade success on aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> e0a121688371bf72e989daeac2fffda5447ae01d

Compatibility test results for aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> e0a121688371bf72e989daeac2fffda5447ae01d (PR)
Upgrade the nodes to version: e0a121688371bf72e989daeac2fffda5447ae01d
framework_upgrade::framework-upgrade::full-framework-upgrade : 6148 TPS, 6392 ms latency, 9100 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> e0a121688371bf72e989daeac2fffda5447ae01d passed
Test Ok

@gerben-stavenga gerben-stavenga merged commit 2232b90 into main Apr 14, 2023
@gerben-stavenga gerben-stavenga deleted the gerben-test-view branch April 14, 2023 18:15
0o-de-lally pushed a commit to 0o-de-lally/zapatos that referenced this pull request May 25, 2023
@gerben-stavenga @0o-de-lally
test view function with struct arguments and generic type parameters (a…
f9f996d 
…ptos-labs#7761)

* add test for view

* Add test for generic view struct parameter

* Remove api tests

* small fix

* fmt

* fmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@movekevin movekevin movekevin approved these changes

@xindingw xindingw xindingw approved these changes

@gregnazario gregnazario Awaiting requested review from gregnazario

@0xmaayan 0xmaayan Awaiting requested review from 0xmaayan

@davidiw davidiw Awaiting requested review from davidiw

@lightmark lightmark Awaiting requested review from lightmark

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gerben-stavenga @movekevin @xindingw