-
Notifications
You must be signed in to change notification settings - Fork 22
Jolteon e2e tests #965
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.
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
base: master
Are you sure you want to change the base?
Jolteon e2e tests #965
Conversation
a8cd7b3
to
0994fe7
Compare
e2e-tests/src/substrate_api.py
Outdated
substrate_url = config.nodes_config.node.url | ||
|
||
# Handle different URL schemes and convert to appropriate WebSocket scheme | ||
if substrate_url.startswith('https://'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use pattern matching here
block = self.cardano_cli.get_block() | ||
logger.debug(f"Current main chain block: {block}") | ||
return block | ||
if self.cardano_cli: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to move refactoring and improvement of the current e2e tests in a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I need this code because tachyeon is not using cardano-cli, so I need to check for it first, or else these tests won't work for tachyeon.
@@ -0,0 +1,156 @@ | |||
# Jolteon Consensus Testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leave one doc - this one or e2e-tests/IMPLEMENTATION_SUMMARY.
|
||
## References | ||
|
||
- [Tachyeon Test Cases Document](../docs/tachyeon-test-cases.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this file exist?
pytest tests/ -m jolteon --blockchain substrate | ||
``` | ||
|
||
## Test Implementation Strategy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strategies needs to be moved out of the docs. Keep general info about custom tags and how to run cases
except Exception as e: | ||
logger.warning(f"Failed to get partner chain status, using mock data: {e}") | ||
# Mock status for standard Substrate nodes | ||
session.config.partner_chain_status = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also can be moved into PR with enhancements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Alex, this is another case where I need this conditional for tacheon tests because there's no partner-chain rpc in this case, we need to use substrate rpc api.
Maybe I'm misunderstanding something here.
@@ -0,0 +1,220 @@ | |||
from time import sleep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to keep it in the PR?
@@ -0,0 +1,94 @@ | |||
from pytest import mark |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as for test_jolteon_simple_debug.py
* get latest substrate block number again | ||
* verify that block numbers increased | ||
""" | ||
# Get initial block using the get_block method which returns block info including number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can omit self-evident comments, because Python code is readable by itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How this test differs from out regular test_smoke.py?
logger.info(f"Initial block number: {initial_block}") | ||
|
||
# Wait for block production (standard Substrate usually produces blocks every 6 seconds) | ||
sleep_time = 15 # Wait 15 seconds to ensure at least one block is produced |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move hardcoded value into constants or read those constants from chain config
assert final_block > initial_block, f"No new blocks produced. Initial: {initial_block}, Final: {final_block}" | ||
|
||
@mark.test_key('SUBSTRATE-002') | ||
@mark.xdist_group("faucet_tx") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this group?
# Use substrate interface's built-in transfer method to avoid custom transaction building issues | ||
try: | ||
# Create call manually | ||
call = api.substrate.compose_call( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we extract a method into substrate_api? So it will be reusable for others
logger.info(f"Health: {health}") | ||
|
||
# Basic assertions | ||
assert chain_name is not None, "Chain name should not be None" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better place assertions before logging. Thus if any value is None - logs will not be populated with it.
- Round numbers are advancing as expected | ||
- Blocks are being certified with proper consensus | ||
|
||
Based on Tachyeon test case: Core Protocol Functionality - Happy Path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove reference to the document
class TestJolteonConsensus: | ||
"""Test cases for Jolteon consensus protocol implementation (Tachyeon)""" | ||
|
||
@mark.test_key('JOLTEON-001') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we created corresponding xray issue for each of the case?
|
||
# Wait for multiple blocks to be produced and certified | ||
# Jolteon typically needs multiple rounds to form QCs and advance | ||
wait_time = 30 # Wait 30 seconds to see multiple blocks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read constants for round time from chaing config and clear calculation that we are waiting for two rounds
# Get final block info | ||
final_block_info = api.substrate.get_block() | ||
final_block_number = final_block_info['header']['number'] | ||
final_round = self._extract_round_number(final_block_info) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is great that you have extracted it as a separate method! We can consider moving it to general substrate_api or create a child class of substrate_api with custom consensus methods
f"No new blocks produced. Initial: {initial_block_number}, Final: {final_block_number}" | ||
|
||
# Verify round advancement (if round information is available) | ||
if initial_round is not None and final_round is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is initial_round or final round are None? We need to assert it before using.
|
||
# Verify round advancement (if round information is available) | ||
if initial_round is not None and final_round is not None: | ||
assert final_round >= initial_round, \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 50-57 seems to check the same thing. Please leave one assertion.
|
||
# Verify block production rate is reasonable | ||
blocks_produced = final_block_number - initial_block_number | ||
expected_min_blocks = wait_time // 6 # Assuming ~6 second block time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove hardcoded value. Get it from config file.
assert blocks_produced >= expected_min_blocks, \ | ||
f"Expected at least {expected_min_blocks} blocks in {wait_time}s, got {blocks_produced}" | ||
|
||
logger.info(f"✅ Jolteon consensus test passed: {blocks_produced} blocks produced") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need add is as an info. Maybe debug level logs or remove it.
for authority in authorities: | ||
assert len(authority) == 32, f"Authority should be 32 bytes, got {len(authority)}" | ||
|
||
logger.info(f"✅ Consensus authorities test passed: {len(authorities)} authorities found") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant logs.
|
||
# Check if authorities are in expected format (AccountId32) | ||
for authority in authorities: | ||
assert len(authority) == 32, f"Authority should be 32 bytes, got {len(authority)}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we checking only that authorities are present? What about leader rotation?
logger.warning(f"Could not retrieve authorities (may not be implemented yet): {e}") | ||
# This is not a failure - the test infrastructure might not support this yet | ||
|
||
def _extract_round_number(self, block_info): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move internal method to the bottom of the file (after tests) - for clarity
return None | ||
|
||
@mark.test_key('JOLTEON-003') | ||
def test_consensus_metadata_availability(self, api: BlockchainApi, config: ApiConfig): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to update this test once we will have a proper data
@@ -0,0 +1,232 @@ | |||
from pytest import mark |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to keep this file?
import logging as logger | ||
|
||
|
||
@mark.jolteon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not forget to add all custom tags into pytest.ini and in the docs (describing what it for)
…docker environment - Fix SubstrateApi WebSocket URL construction to properly convert HTTP/HTTPS to WS/WSS with /ws path - Replace Kubernetes runners with local runners in jolteon_docker stack config to avoid k8s dependency - Add Alice development account (//Alice) fixture for jolteon_docker environment with sufficient balance - Create comprehensive substrate smoke tests covering block production, node info, balance queries, and transactions - Add graceful handling of solochain template runtime validation issues in transaction tests - Improve API resilience with mock fallbacks for missing partner chain components Tests now successfully validate connectivity, RPC functionality, and basic operations against jolteon_docker substrate node. Transaction validation fails due to runtime WASM issue in TaggedTransactionQueue_validate_transaction.
- Add hardcoded Polkadot SDK-compatible type registry for Jolteon runtime - Upgrade substrate-interface to 1.7.11 and scalecodec to 1.2.11 - Remove skip-on-runtime-errors behavior from transaction test - Resolves WASM runtime validation errors and codec decoding issues
a202dba
to
535d215
Compare
* new config files for jolteon docker, connectivity only * tests: update node files and secrets with node keys * tests: add secrets config for jolteon_docker env * Fix WebSocket connectivity and add substrate smoke tests for jolteon_docker environment - Fix SubstrateApi WebSocket URL construction to properly convert HTTP/HTTPS to WS/WSS with /ws path - Replace Kubernetes runners with local runners in jolteon_docker stack config to avoid k8s dependency - Add Alice development account (//Alice) fixture for jolteon_docker environment with sufficient balance - Create comprehensive substrate smoke tests covering block production, node info, balance queries, and transactions - Add graceful handling of solochain template runtime validation issues in transaction tests - Improve API resilience with mock fallbacks for missing partner chain components Tests now successfully validate connectivity, RPC functionality, and basic operations against jolteon_docker substrate node. Transaction validation fails due to runtime WASM issue in TaggedTransactionQueue_validate_transaction. * Fix substrate transaction encoding for Polkadot SDK compatibility - Add hardcoded Polkadot SDK-compatible type registry for Jolteon runtime - Upgrade substrate-interface to 1.7.11 and scalecodec to 1.2.11 - Remove skip-on-runtime-errors behavior from transaction test - Resolves WASM runtime validation errors and codec decoding issues * add more jolteon tests * jolteon: local env test config --------- Co-authored-by: chrispalaskas <[email protected]>
* new config files for jolteon docker, connectivity only * tests: update node files and secrets with node keys * tests: add secrets config for jolteon_docker env * Fix WebSocket connectivity and add substrate smoke tests for jolteon_docker environment - Fix SubstrateApi WebSocket URL construction to properly convert HTTP/HTTPS to WS/WSS with /ws path - Replace Kubernetes runners with local runners in jolteon_docker stack config to avoid k8s dependency - Add Alice development account (//Alice) fixture for jolteon_docker environment with sufficient balance - Create comprehensive substrate smoke tests covering block production, node info, balance queries, and transactions - Add graceful handling of solochain template runtime validation issues in transaction tests - Improve API resilience with mock fallbacks for missing partner chain components Tests now successfully validate connectivity, RPC functionality, and basic operations against jolteon_docker substrate node. Transaction validation fails due to runtime WASM issue in TaggedTransactionQueue_validate_transaction. * Fix substrate transaction encoding for Polkadot SDK compatibility - Add hardcoded Polkadot SDK-compatible type registry for Jolteon runtime - Upgrade substrate-interface to 1.7.11 and scalecodec to 1.2.11 - Remove skip-on-runtime-errors behavior from transaction test - Resolves WASM runtime validation errors and codec decoding issues * add more jolteon tests * jolteon: local env test config * jolteon: rpc e2e tests * feat: replace hardcoded timeouts with configurable Jolteon parameters Replace hardcoded timeouts in Jolteon tests with block_duration-based configurable parameters for better environment flexibility. * refactor(jolteon): replace time-based monitoring with block-based consensus analysis * refactor: improve Jolteon consensus tests with block-based analysis and time-based monitoring - Refactor consensus tests from time-based waiting to efficient block-based analysis - Add shared helper function for consensus state collection across multiple RPC endpoints - Fix test_jolteon_advanced.py safety properties test to require minimum 3 blocks for meaningful validation - Rewrite test_jolteon_consensus_rpc.py with proper syntax and time-based progression monitoring - Update pytest.ini to register custom markers (jolteon, consensus, advanced) - Improve error handling and logging for expected behaviors (duplicate block samples) - Ensure tests properly validate consensus progression over time rather than static state * fix: update Jolteon consensus tests to use time-based monitoring --------- Co-authored-by: chrispalaskas <[email protected]>
8b265b1
to
5de3f49
Compare
5de3f49
to
bfcbcc5
Compare
bfcbcc5
to
08d5fcf
Compare
08d5fcf
to
bfcbcc5
Compare
Description
E2E tests for the Jolteon Environment
Checklist
changelog.md
for affected crate