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.

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

[Test]: Unstable test waiting time #596

Open
1 task done
iGxnon opened this issue Jan 2, 2024 · 0 comments
Open
1 task done

[Test]: Unstable test waiting time #596

iGxnon opened this issue Jan 2, 2024 · 0 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@iGxnon
Copy link
Contributor

iGxnon commented Jan 2, 2024

Description about the bug

In testing, we extensively use sleep(...) to ensure that the Curp server has accomplished a certain task.

read_state.rs

#[tokio::test(flavor = "multi_thread")]
#[abort_on_panic]
async fn read_state() {
    init_logger();
    let group = CurpGroup::new(3).await;
    let put_client = group.new_client().await;
    let put_cmd = TestCommand::new_put(vec![0], 0).set_exe_dur(Duration::from_millis(100));
    tokio::spawn(async move {
        assert_eq!(
            put_client.propose(put_cmd, true).await.unwrap().0,
            TestCommandResult::default(),
        );
    });


    sleep_millis(10).await;


    let get_client = group.new_client().await;
    let res = get_client
        .fetch_read_state(&TestCommand::new_get(vec![0]))
        .await
        .unwrap();
    if let ReadState::Ids(v) = res {
        assert_eq!(v.len(), 1);
    } else {
        unreachable!(
            "expected result should be ReadState::Ids(v) where len(v) = 1, but received {:?}",
            res
        );
    }

    sleep_millis(500).await;

    let res = get_client
        .fetch_read_state(&TestCommand::new_get(vec![0]))
        .await
        .unwrap();
    if let ReadState::CommitIndex(index) = res {
        assert_eq!(index, 1);
    } else {
        unreachable!(
            "expected result should be ReadState::CommitIndex({:?}), but received {:?}",
            1, res
        );
    }
}

Sometimes, introducing some new features may prevent the corresponding task from being completed within the waiting time, leading to the failure of subsequent assertions.

Using sleep is not entirely bad; its failure, from another perspective, is that the processing time for tasks has increased. It is very likely that this new feature will result in performance loss.

However, we should not fail the test to keep us busy fixing it. cargo-nextest provides a slow test detection mechanism that can help us analyze the overall runtime of a specific test.

We should enhance the system's exposure capability to the outside, rather than using sleep for synchronization.

Version

0.1.0

Relevant log output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@iGxnon iGxnon added the bug Something isn't working label Jan 2, 2024
@iGxnon iGxnon added help wanted Extra attention is needed and removed bug Something isn't working labels Jan 15, 2024
@Phoenix500526 Phoenix500526 added the enhancement New feature or request label Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants