Fix test matrix

[Serock3]

---

This change is 

[linear]

DES-1468 `test_upgrade_app` is missing from the test result matrix

[MarkusPettersson98]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Serock3)

---

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

pub fn should_run_on_os(targets: &[Os], os: Os) -> bool {
    targets.is_empty() || targets.contains(&os)
}

Would it make sense to tie this to the TestDescription struct? There is a logical connection to a specific test whenever should_run_on_os is invoked I presume, but currently that connection is not expressed in this type signature 😊

Code quote:

pub fn should_run_on_os(targets: &[Os], os: Os) -> bool {
    targets.is_empty() || targets.contains(&os)
}

---

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

        name: "test_upgrade_app",
        targets: &[],
    };

⛏️ Would it make sense to briefly explain why this test is added here, and this workaround could potentially be removed in the future? 😊

Code quote:

    let test_upgrade_app = TestDesciption {
        priority: None,
        name: "test_upgrade_app",
        targets: &[],
    };

[Serock3]

Reviewable status: 2 of 6 files reviewed, 1 unresolved discussion (waiting on @MarkusPettersson98)

---

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

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Would it make sense to tie this to the TestDescription struct? There is a logical connection to a specific test whenever should_run_on_os is invoked I presume, but currently that connection is not expressed in this type signature 😊

Some context: I added TestDescription to avoid having to instantiateTestMetadata for test_upgrade_app (see the the first commit). It contained parameters that I didn't want to provide, like a function handle.

This function was tied to the TestMetadata before, but now it needs to be available to both it and TestDescription. I would need a trait, or to duplicate the method, to attach it to the types.

---

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

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

⛏️ Would it make sense to briefly explain why this test is added here, and this workaround could potentially be removed in the future? 😊

Sure, I added some documentation. Needing TestDescription in the first place is kind of a hack, but the proper solution needs a larger refactoring. I elaborated on that in a comment.

[MarkusPettersson98]

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

---

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

Previously, Serock3 (Sebastian Holmin) wrote…

Some context: I added TestDescription to avoid having to instantiateTestMetadata for test_upgrade_app (see the the first commit). It contained parameters that I didn't want to provide, like a function handle.

This function was tied to the TestMetadata before, but now it needs to be available to both it and TestDescription. I would need a trait, or to duplicate the method, to attach it to the types.

Hmm, sad :( Oh well, it is what it is 😊

[MarkusPettersson98]

Reviewed 1 of 6 files at r3, 1 of 3 files at r4, 1 of 3 files at r5, 4 of 4 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved