Skip to content

Integrate existing executor tests with the new testing framework#1342

Merged
NingLin-P merged 9 commits into
mainfrom
executor-tests
Apr 7, 2023
Merged

Integrate existing executor tests with the new testing framework#1342
NingLin-P merged 9 commits into
mainfrom
executor-tests

Conversation

@NingLin-P

@NingLin-P NingLin-P commented Mar 31, 2023

Copy link
Copy Markdown
Contributor

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 reviewed Apr 1, 2023
View reviewed changes
Comment thread domains/client/domain-executor/src/tests.rs Outdated
Comment thread domains/client/domain-executor/src/tests.rs Outdated
@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

@vedhavyas vedhavyas left a comment

Copy link
Copy Markdown
Contributor

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

NingLin-P commented Apr 4, 2023

Copy link
Copy Markdown
Contributor 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

NingLin-P commented Apr 6, 2023

Copy link
Copy Markdown
Contributor 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

@vedhavyas vedhavyas vedhavyas left review comments

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

@rg3l3dr rg3l3dr Awaiting requested review from rg3l3dr

+1 more reviewer

@liuchengxu liuchengxu liuchengxu approved these changes

Reviewers whose approvals may not affect merge requirements

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