Structure error handling in test_automatic_wireguard_rotation

[MarkusPettersson98]

This PR overhauls test_automatic_wireguard_rotation to use anyhow to propagate error values instead of panicking, which is a commonly used strategy in other end-to-end tests. These are the fruits of some work investigating flakiness related to the test.

---

This change is 

[linear]

DES-1094 `test_automatic_wireguard_rotation` fails due to timeout

[Serock3]

Reviewed 1 of 1 files at r1, 1 of 1 files at r2, 4 of 4 files at r3, 1 of 1 files at r4, 1 of 1 files at r5, 4 of 4 files at r6, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @MarkusPettersson98)

---

test/test-manager/src/run_tests.rs line 95 at r3 (raw file):

        }

        // TODO: Log how long each test took to run.

This comment is included in commit 64bb485

---

test/test-manager/src/run_tests.rs line 141 at r6 (raw file):

                        .await
                {
                    log::error!("Clean up between tests failed");

This may cause merge conflict with test-manager-fixes, just FYI

---

test/test-manager/src/tests/account.rs line 315 at r1 (raw file):

    // listen for device daemon events until we observe the change. We have to register the event
    // listener before polling the current key to be sure we don't miss the change.
    log::info!("Verifying that wireguard key has change");

Nit: maybe fix spelling mistake while at it change -> changed

---

test/test-manager/src/tests/mod.rs line 84 at r3 (raw file):

    log::debug!("Cleaning up daemon in test cleanup");
    // Check if daemon should be restarted.
    // TODO: The deamon needs to be up and running after this line.

Maybe rebase on test-manager-fixes, or wait for that PR to be merged

---

test/test-rpc/src/client.rs line 266 at r3 (raw file):

    /// blocking execution until then.
    pub async fn stop_mullvad_daemon(&self) -> Result<(), Error> {
        // TODO: Increase timeout and log how long it took (?)

Don't forget to remove

---

test/test-rpc/src/client.rs line 391 at r6 (raw file):

        self.connection_handle.wait_for_server().await?;

        // TODO: Get rid of this magic number.

Don't forget the TODO

[Serock3]

Reviewed 2 of 7 files at r7, 9 of 9 files at r8, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @MarkusPettersson98)

[MarkusPettersson98]

Reviewable status: 8 of 9 files reviewed, 3 unresolved discussions (waiting on @Serock3)

---

test/test-manager/src/run_tests.rs line 95 at r3 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

This comment is included in commit 64bb485

Yes

---

test/test-manager/src/run_tests.rs line 141 at r6 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

This may cause merge conflict with test-manager-fixes, just FYI

Yeah, let's merge that first. I'll deal with the mess later :)

---

test/test-manager/src/tests/account.rs line 315 at r1 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

Nit: maybe fix spelling mistake while at it change -> changed

Done

---

test/test-manager/src/tests/mod.rs line 84 at r3 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

Maybe rebase on test-manager-fixes, or wait for that PR to be merged

Awaiting that PR to be merged

---

test/test-rpc/src/client.rs line 391 at r6 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

Don't forget the TODO

Done

[MarkusPettersson98]

Reviewable status: 0 of 12 files reviewed, 4 unresolved discussions (waiting on @Serock3)

---

test/test-manager/src/tests/account.rs line 265 at r11 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

I think I like my version of this comment better (used to be above the creation of the event listener). Was there something about it you disagreed with?

Added it back, I don't see the point in not having it there tbh 😊

---

test/test-manager/src/tests/mod.rs line 83 at r11 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

I'd avoid touching this fn if I were you, it's going to conflict with my test-manager-fixes PR

Yup, moving those changes to a separate PR

[Serock3]

Reviewed 1 of 1 files at r9, 2 of 4 files at r10, 10 of 12 files at r11, 2 of 2 files at r12, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved