test(apollo_network): unit tests for the EventWakerManager struct

[guyf-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[guyf-starkware]

• chore(apollo_network): remove unneccasry multi wakers functionality and add a test #6702
• test(apollo_network): unit tests for the TimeWakerManager struct #6701
• test(apollo_network): unit tests for the EventWakerManager struct #6647 👈 (View in Graphite)
• main-v0.14.0

This stack of pull requests is managed by Graphite. Learn more about stacking.

[github-actions]

Artifacts upload workflows:

• Started at 2025-05-21T13:45:50Z
• Started at 2025-05-21T14:35:09Z
• Started at 2025-05-21T14:39:10Z
• Started at 2025-05-22T06:41:51Z

[github-actions]

Benchmark movements: No major performance changes detected.

[ShahakShama]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @guyf-starkware)

---

Cargo.toml line 313 at r1 (raw file):

validator = "0.12"
void = "1.0.2"
waker-fn = "1"

I think we don't usually write versions with just the major. I'm not sure but I think the meaning of this is to bring any version of the form 1..
Please change this to the version you want (e.g 1.0.0)

---

crates/apollo_network/src/discovery/behaviours/event_waker_manager_test.rs line 16 at r1 (raw file):

#[test]
fn wakes_all_wakers() {
    let waker_1_was_called = Arc::new(AtomicBool::new(false));

I remember today's talk on "sometimes it's worth not extracting code to function in unit tests", but IMO this is not the case and you should create a struct called MockWaker and have 3 functions for it: waker, is_woken, reset

---

crates/apollo_network/src/discovery/behaviours/event_waker_manager_test.rs line 55 at r1 (raw file):

    assert!(waker_was_called.load(Ordering::SeqCst));
    // Set the value back to false to see it doesn't get set to true by the waker call.

Wouldn't it be easier to use integer instead of boolean and to do increments instead of setting to true?

[guyf-starkware]

Reviewable status: 2 of 6 files reviewed, 3 unresolved discussions (waiting on @ShahakShama)

---

crates/apollo_network/src/discovery/behaviours/event_waker_manager_test.rs line 16 at r1 (raw file):

Previously, ShahakShama wrote…

I remember today's talk on "sometimes it's worth not extracting code to function in unit tests", but IMO this is not the case and you should create a struct called MockWaker and have 3 functions for it: waker, is_woken, reset

Done except the reset, it's not needed once we moved to counting

---

crates/apollo_network/src/discovery/behaviours/event_waker_manager_test.rs line 55 at r1 (raw file):

Previously, ShahakShama wrote…

Wouldn't it be easier to use integer instead of boolean and to do increments instead of setting to true?

Done

[ShahakShama]

Reviewed 3 of 4 files at r2, all commit messages.
Reviewable status: 5 of 6 files reviewed, 2 unresolved discussions

---

crates/apollo_network/src/discovery/behaviours/test_util.rs line 1 at r2 (raw file):

use std::sync::atomic::{AtomicUsize, Ordering};

Consider moving this to crates/apollo_network/src/test_utils/mod.rs

---

crates/apollo_network/src/discovery/behaviours/test_util.rs line 17 at r2 (raw file):

        let times_woken_clone = times_woken.clone();
        let waker = waker_fn(move || {
            times_woken_clone.fetch_add(1, Ordering::SeqCst);

Move the ordering to a constant

[guyf-starkware]

Reviewable status: 5 of 6 files reviewed, 2 unresolved discussions (waiting on @ShahakShama)

---

Cargo.toml line 313 at r1 (raw file):

Previously, ShahakShama wrote…

I think we don't usually write versions with just the major. I'm not sure but I think the meaning of this is to bring any version of the form 1..
Please change this to the version you want (e.g 1.0.0)

Not sure if this is a new comment or I somehow forgot to say Done on the previous one, Eiter way it's now 1.2.0 (not jsut hte major)

---

crates/apollo_network/src/discovery/behaviours/test_util.rs line 1 at r2 (raw file):

Previously, ShahakShama wrote…

Consider moving this to crates/apollo_network/src/test_utils/mod.rs

I'm going to use it in the other event manager test as well, do you still want me to move it?

---

crates/apollo_network/src/discovery/behaviours/test_util.rs line 17 at r2 (raw file):

Previously, ShahakShama wrote…

Move the ordering to a constant

This is considered the default for tests but i dont mind moving it.