Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Conversation

@lexnv
Copy link
Contributor

@lexnv lexnv commented Mar 22, 2023

This PR checks the correct behavior of the chainHead RPC API when the
FinalizedNotification is received before the BlockImportNotification.

To achieve this a ChainHeadMockClient mocks the BlockchainEvents trait to ensure
that tests can trigger either FinalityNotification or BlockImportNotification on demand.
Other traits imposed by the bounds of the ChainHead are implemented as pass-through.

While at it, increase the timeout duration of the get_next_event() that awaits for chainHead
events in the testing environment.

This is a follow-up for the fix #13632.

// CC @paritytech/tools-team

@lexnv lexnv added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Mar 22, 2023
@lexnv lexnv requested review from bkchr, jsdw, niklasad1 and skunert March 22, 2023 13:16
@lexnv lexnv self-assigned this Mar 22, 2023
Copy link
Contributor

@jsdw jsdw left a comment

Choose a reason for hiding this comment

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

Makes sense, looks good! Loads of boilerplate to have the mock client but probably not easily avoidable :)

@lexnv lexnv merged commit 988f6ad into master Mar 23, 2023
@lexnv lexnv deleted the lexnv/chainhead_notif_tests branch March 23, 2023 13:40
breathx pushed a commit to gear-tech/substrate that referenced this pull request Apr 22, 2023
…ytech#13680)

* chain_head/tests: Mock client for custom block notification

Signed-off-by: Alexandru Vasile <[email protected]>

* chain_head/tests: Check finalized block event before new block

Signed-off-by: Alexandru Vasile <[email protected]>

* Update client/rpc-spec-v2/src/chain_head/test_utils.rs

Co-authored-by: Bastian Köcher <[email protected]>

* Update client/rpc-spec-v2/src/chain_head/test_utils.rs

Co-authored-by: Bastian Köcher <[email protected]>

* Update client/rpc-spec-v2/src/chain_head/test_utils.rs

Co-authored-by: Bastian Köcher <[email protected]>

* chain_head/tests: Run import events with 10min timeout

Signed-off-by: Alexandru Vasile <[email protected]>

* chain_head/tests: Add comments about test

Signed-off-by: Alexandru Vasile <[email protected]>

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
…ytech#13680)

* chain_head/tests: Mock client for custom block notification

Signed-off-by: Alexandru Vasile <[email protected]>

* chain_head/tests: Check finalized block event before new block

Signed-off-by: Alexandru Vasile <[email protected]>

* Update client/rpc-spec-v2/src/chain_head/test_utils.rs

Co-authored-by: Bastian Köcher <[email protected]>

* Update client/rpc-spec-v2/src/chain_head/test_utils.rs

Co-authored-by: Bastian Köcher <[email protected]>

* Update client/rpc-spec-v2/src/chain_head/test_utils.rs

Co-authored-by: Bastian Köcher <[email protected]>

* chain_head/tests: Run import events with 10min timeout

Signed-off-by: Alexandru Vasile <[email protected]>

* chain_head/tests: Add comments about test

Signed-off-by: Alexandru Vasile <[email protected]>

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants