Radish alpha
h
heartwood
rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5
Radicle Heartwood Protocol & Stack
Radicle
Git
Patches › b232f2f1
e2e: Fix flakey tests
Merged ade opened 9 days ago

There are e2e tests that are causing breaks in slower environments like CI:

  • test_connection_crossing
  • test_non_fastforward_identity_doc
Activity Changes +46 -18
4 files changed +46 -18 bbb12796 → bdd991a5
modified crates/radicle-cli/examples/rad-patch-fetch-2.md
@@ -22,7 +22,6 @@ $ git branch -r
$ git pull

Already up to date.

$ git branch -r
- 
  rad/HEAD -> rad/master

  rad/master

  rad/patches/5e2dedcc5d515fcbc1cca483d3376609fe889bfb

```
modified crates/radicle-cli/examples/rad-patch-via-push.md
@@ -62,7 +62,6 @@ And let's look at our local and remote refs:
$ git show-ref

42d894a83c9c356552a57af09ccdbd5587a99045 refs/heads/feature/1

f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354 refs/heads/master
- 
f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354 refs/remotes/rad/HEAD

f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354 refs/remotes/rad/master

42d894a83c9c356552a57af09ccdbd5587a99045 refs/remotes/rad/patches/6035d2f582afbe01ff23ea87528ae523d76875b6

```
modified crates/radicle-node/src/tests/e2e.rs
@@ -873,10 +873,14 @@ fn test_connection_crossing() {


    log::debug!(target: "test", "Preferred peer is {preferred}");
+ 
    let barrier = std::sync::Arc::new(std::sync::Barrier::new(2));
+ 
    let b1 = barrier.clone();
+ 
    let b2 = barrier.clone();

    let t1 = thread::spawn({

        let mut alice = alice.handle.clone();



        move || {
+ 
            b1.wait();

            alice

                .connect(bob.id, bob.addr.into(), ConnectOptions::default())

                .unwrap()
@@ -885,6 +889,7 @@ fn test_connection_crossing() {
    let t2 = thread::spawn({

        let mut bob = bob.handle.clone();

        move || {
+ 
            b2.wait();

            bob.connect(alice.id, alice.addr.into(), ConnectOptions::default())

                .unwrap()

        }
@@ -901,25 +906,36 @@ fn test_connection_crossing() {
        assert_matches!(r2, ConnectResult::Connected);

    }
- 
    thread::sleep(time::Duration::from_secs(1));
+ 
    let mut iterations = 0;
+ 
    let (alice_s, bob_s, s1, s2) = loop {
+ 
        let alice_s = alice.handle.sessions().unwrap();
+ 
        let bob_s = bob.handle.sessions().unwrap();
- 
    let alice_s = alice.handle.sessions().unwrap();
- 
    let bob_s = bob.handle.sessions().unwrap();
+ 
        let s1 = alice_s.iter().find(|s| s.nid == bob.id).cloned();
+ 
        let s2 = bob_s.iter().find(|s| s.nid == alice.id).cloned();
- 
    // Both sessions are established.
- 
    let s1 = alice_s.iter().find(|s| s.nid == bob.id).unwrap();
- 
    let s2 = bob_s.iter().find(|s| s.nid == alice.id).unwrap();
-
- 
    log::debug!(target: "test", "{:?}", alice.handle.sessions());
- 
    log::debug!(target: "test", "{:?}", bob.handle.sessions());
+ 
        if let (Some(s1), Some(s2)) = (s1, s2) {
+ 
            // Wait until both sessions are fully connected
+ 
            if s1.state.is_connected() && s2.state.is_connected() {
+ 
                break (alice_s, bob_s, s1, s2);
+ 
            }
+ 
        }
+ 
        iterations += 1;
+ 
        if iterations >= 100 {
+ 
            panic!("Timeout waiting for sessions to connect");
+ 
        }
+ 
        thread::sleep(time::Duration::from_millis(50));
+ 
    };
- 
    if preferred == alice.id {
- 
        assert_eq!(s1.link, radicle::node::Link::Outbound);
- 
        assert_eq!(s2.link, radicle::node::Link::Inbound);
- 
    } else {
- 
        assert_eq!(s1.link, radicle::node::Link::Inbound);
- 
        assert_eq!(s2.link, radicle::node::Link::Outbound);
- 
    }
+ 
    // We assert that they have opposite link directions.
+ 
    // In a true simultaneous crossing, the preferred peer wins the Outbound link.
+ 
    // However, due to OS thread scheduling and reactor event ordering, one peer
+ 
    // might fully establish the connection before the other even processes the dial command.
+ 
    // In all valid cases (crossing or sequential), exactly one is Outbound and one is Inbound.
+ 
    assert_ne!(
+ 
        s1.link, s2.link,
+ 
        "One must be Inbound and the other Outbound"
+ 
    );

    assert_eq!(alice_s.len(), 1);

    assert_eq!(bob_s.len(), 1);

}
@@ -1709,6 +1725,7 @@ fn test_non_fastforward_identity_doc() {
    let mut alice = alice.spawn();

    let mut alice_laptop = alice_laptop.spawn();

    let mut bob = bob.spawn();
+ 
    let bob_events = bob.handle.events();

    let mut eve = eve.spawn();



    let has_issue = |node: &NodeHandle<MockSigner>, issue: &cob::ObjectId| -> bool {
@@ -1754,6 +1771,13 @@ fn test_non_fastforward_identity_doc() {
        .unwrap();

    assert!(has_issue(&eve, &issue));
+ 
    bob_events
+ 
        .wait(
+ 
            |e| matches!(e, Event::RefsAnnounced { nid, .. } if *nid == eve.id).then_some(()),
+ 
            DEFAULT_TIMEOUT,
+ 
        )
+ 
        .unwrap();
+ 


    // Alice updates the identity of the document to include her laptop

    let (prev, next) = {

        let repo = alice.storage.repository(rid).unwrap();
modified crates/radicle/src/test/fixtures.rs
@@ -122,6 +122,12 @@ fn repository_with<P: AsRef<Path>>(
        let mut config = repo.config().unwrap();

        config.set_str("user.name", USER_NAME).unwrap();

        config.set_str("user.email", USER_EMAIL).unwrap();
+ 
        // Git older than v2.48.0 have the behaviour of 'remote.<remote>.followRemoteHEAD=never',
+ 
        // So we set it explicitly here for testing purposes, for consistent output of commands
+ 
        // like 'git branch -r' and 'git show-ref'
+ 
        config
+ 
            .set_str("remote.rad.followRemoteHEAD", "never")
+ 
            .unwrap();

    }



    let sig = git::raw::Signature::new(