[test] baseline failures #15632
[test] baseline failures #15632
Conversation
danielxiangzl
added
the
CICD:build-failpoints-images
⏱️ 3h 18m total CI duration on this PR
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| ) | ||
| .await | ||
| .unwrap(); | ||
| panic!("test_fault_tolerance_of_leader_equivocation"); |
There was a problem hiding this comment.
This panic! statement causes the test to fail unconditionally, regardless of whether the test logic succeeds or fails. Since the test appears to be validating leader equivocation fault tolerance, the test should be allowed to complete normally and verify the expected behavior through its assertions.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
| #[async_trait] | ||
| impl NetworkLoadTest for PerformanceBenchmark { | ||
| async fn test( | ||
| &self, | ||
| swarm: Arc<tokio::sync::RwLock<Box<dyn Swarm>>>, | ||
| _report: &mut TestReport, | ||
| duration: Duration, | ||
| ) -> Result<()> { | ||
| let validators = { swarm.read().await.get_validator_clients_with_names() }; | ||
| // 10 vals, test 1,2,3 failures | ||
| let num_bad_leaders = 1; | ||
| for (name, validator) in validators[..num_bad_leaders].iter() { | ||
| validator | ||
| .set_failpoint( | ||
| "consensus::leader_equivocation".to_string(), | ||
| "return".to_string(), | ||
| ) | ||
| .await | ||
| .map_err(|e| { | ||
| anyhow!( | ||
| "set_failpoint to set consensus leader equivocation on {} failed, {:?}", | ||
| name, | ||
| e | ||
| ) | ||
| })?; | ||
| }; | ||
| Ok(()) | ||
| } | ||
| } |
There was a problem hiding this comment.
The test should honor the duration parameter by adding tokio::time::sleep(duration).await after setting the failpoints. This ensures the test runs for the expected duration and gives the failpoints time to take effect. Without this sleep, the test may complete prematurely before the fault injection has meaningful impact.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
danielxiangzl
force-pushed
the
daniel-baseline-failures
branch
from
dde8216 to
328f7e8
Compare
| async fn test( | ||
| &self, | ||
| swarm: Arc<tokio::sync::RwLock<Box<dyn Swarm>>>, | ||
| _report: &mut TestReport, | ||
| duration: Duration, | ||
| ) -> Result<()> { | ||
| let validators = { swarm.read().await.get_validator_clients_with_names() }; | ||
| // 10 vals, test 1,2,3 failures | ||
| let num_bad_leaders = 3; | ||
| for (name, validator) in validators[..num_bad_leaders].iter() { | ||
| validator | ||
| .set_failpoint( | ||
| "consensus::leader_equivocation".to_string(), | ||
| "return".to_string(), | ||
| ) | ||
| .await | ||
| .map_err(|e| { | ||
| anyhow!( | ||
| "set_failpoint to set consensus leader equivocation on {} failed, {:?}", | ||
| name, | ||
| e | ||
| ) | ||
| })?; | ||
| }; | ||
| Ok(()) | ||
| } | ||
| } |
There was a problem hiding this comment.
The test currently returns immediately after setting the failpoints, without waiting for the specified duration. This means the test may complete before the injected failures have time to manifest and be observed. Adding tokio::time::sleep(duration).await before returning would ensure the test runs for the intended duration while the failpoints are active.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
danielxiangzl
force-pushed
the
daniel-baseline
branch
from
a492fa9 to
cc7eeb5
Compare
Description
How Has This Been Tested?
Key Areas to Review
Type of Change
Which Components or Systems Does This Change Impact?
Checklist