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

Integrate existing executor tests with the new testing framework #1342

Merged
merged 9 commits into from
Apr 7, 2023
Merged

Integrate existing executor tests with the new testing framework #1342

merged 9 commits into from
Apr 7, 2023

Conversation

NingLin-P
Copy link
Member

This PR integrates all the remaining executor tests with the new test framework.

The only ignored executor test is fraud_proof_verification_in_tx_pool_should_work, to enable it we need to support fetching the call_data of the fraud proof verifier from the original primary block.

Code contributor checklist:

@NingLin-P NingLin-P added the execution Subspace execution label Mar 31, 2023
@NingLin-P NingLin-P added this to the Gemini 3 milestone Mar 31, 2023
@NingLin-P NingLin-P mentioned this pull request Mar 31, 2023
Closed
@liuchengxu liuchengxu mentioned this pull request Apr 2, 2023
Merged
1 task
@NingLin-P NingLin-P force-pushed the executor-tests branch 2 times, most recently from b8ce592 to 4357c41 Compare April 4, 2023 14:22
vedhavyas
vedhavyas previously approved these changes Apr 4, 2023
View reviewed changes
Copy link
Member

@vedhavyas vedhavyas left a comment

Choose a reason for hiding this comment

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

make sense!
Great work!

@NingLin-P
Copy link
Member Author

The test pallet_domains_unsigned_extrinsics_should_work failed in CI due to take too much time, it is also caused by the ack of slot notification never resolves (perhaps the same issue of #1347).

It can be reproduced in my machine, one interesting glitch I found was that the slot notification could be successfully sent to alice's subscriber even after we manually dropped alice in the test then the test blocks on waiting for ack from alice. I will continue investigating this issue.

@NingLin-P
Copy link
Member Author

The test pallet_domains_unsigned_extrinsics_should_work failed in CI due to the same issue of #1347, but with a slight difference that we have manually dropped alice in the test and its domain worker is terminated asynchronously, and the slot notification may be successfully sent to it but never handled and the slot_acknowledgement_sender never drops due to the same reason mentioned at #1352 (comment) thus the test is blocked forever.

NingLin-P added 8 commits April 7, 2023 17:45
@NingLin-P
Replace is_bundle_present_in_tx_pool with get_bundle_from_tx_pool
fc4df6c 
get_bundle_from_tx_pool is used in incoming test and is_bundle_present_in_tx_pool
can be implement by get_bundle_from_tx_pool(..).is_some()

Signed-off-by: linning <linningde25@gmail.com>
@NingLin-P
Add produce_block_with_extrinsics to MockPrimaryNode
581271d 
produce_block_with_extrinsics is useful when we want to control the exact extrinsic
included in block or for extrinsic that may fail on verification but we still want
to include it in block.

This commit also extract the new block logging in a separated function
so both produce_block_with_slot and produce_block_with_extrinsics can reuse it.

Signed-off-by: linning <linningde25@gmail.com>
@NingLin-P
Integrate test execution_proof_creation_and_verification_should_work …
fab9ca9 
…with new testing framework

This test is broken and fixed by adding primary_block_info digest to block header

Signed-off-by: linning <linningde25@gmail.com>
@NingLin-P
Integrate test fraud_proof_verification_in_tx_pool_should_work with t…
d0abe73 
…he new testing framework

The test is still ignored due to we don't support fetch the call_data from the primary chain,
and it will be a ready-made test when we support so, there is also a TODO left to test the
initialize_block fraud proof of block 1 when we can properly handle the genesis receipt on
verify_pre_state_root.

Signed-off-by: linning <linningde25@gmail.com>
@NingLin-P
Integrate test extract_core_domain_wasm_bundle_in_system_domain_runti…
ba1fc90 
…me_should_work with the new testing framework

Signed-off-by: linning <linningde25@gmail.com>
@NingLin-P
Integrate test pallet_domains_unsigned_extrinsics_should_work with th…
2a3a076 
…e new testing framework

Signed-off-by: linning <linningde25@gmail.com>
@NingLin-P
Apply review suggestions
cbf56bb 
Signed-off-by: linning <linningde25@gmail.com>
@NingLin-P
Integrate test set_new_code_should_work with the new testing framework
521062e 
The test is still ignored due to we can not construct a proper primary runtime code.

Signed-off-by: linning <linningde25@gmail.com>
@NingLin-P NingLin-P enabled auto-merge April 7, 2023 14:51
@NingLin-P NingLin-P merged commit df8d027 into main Apr 7, 2023
@NingLin-P NingLin-P deleted the executor-tests branch April 7, 2023 16:39
i1i1 added a commit to autonomys/subspace-pulsar-sdk that referenced this pull request Apr 10, 2023
@i1i1
Import autonomys/subspace#1357,autonomys/subspace#1360,autonomys/subs…
8fdad6c 
…pace#1342,autonomys/subspace#1361,autonomys/subspace#1359
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liuchengxu liuchengxu liuchengxu approved these changes

@vedhavyas vedhavyas vedhavyas left review comments

@nazar-pc nazar-pc Awaiting requested review from nazar-pc

@rg3l3dr rg3l3dr Awaiting requested review from rg3l3dr

Assignees
No one assigned
Labels
execution Subspace execution
Projects
Execution Domains
Archived in project
Milestone
Gemini 3
Development

Successfully merging this pull request may close these issues.
3 participants
@NingLin-P @vedhavyas @liuchengxu