Marker inheritance and non-parametrised tests with solver dependency

[ThomSerg]

The build-in helpers of PyTest giving easy access to the markers all seem to merge markers when working with inheritance; a subclass also gets the markers from its superclass, even if marker had been redeclared. So had to more directly access the pytestmark on the bare class.

# requires_solver from the function itself, or from the concrete collected class.
class_marks = getattr(metafunc.cls, "__dict__", {}).get("pytestmark", []) if metafunc.cls else []
requires_solver_marker = next((m for m in metafunc.definition.own_markers + class_marks if m.name == "requires_solver"), None)

Additionally had to add skipping logic for tests that have a solver dependency but that do not actually take a solver argument as input (i.e. solver argument is hard-coded).

@pytest.mark.requires_solver("gurobi")
class TestNativeMusGurobi(TestMus):
    def setup_method(self):  # <- uses Gurobi but does not take solver parametrisation
        self.mus_func = lambda soft, hard=[], solver="gurobi": mus_native(soft, hard=hard, solver="gurobi")
        self.naive_func = mus_naive

An alternative (that does not require added skip logic):

@pytest.mark.requires_solver("gurobi")
class TestNativeMusGurobi(TestMus):
    def setup_method(self, solver): # <- now does take parametrisation, but will always be gurobi
        self.mus_func = lambda soft, hard=[], solver=solver: mus_native(soft, hard=hard, solver="gurobi")
        self.naive_func = mus_naive

So only a question about which of the above two is preferred.

[OrestisLomis]

I would say the second is preferred, it allows for more dynamically adding more solvers to the tests (as done in #950).

[ThomSerg]

But the test is named TestNativeMusGurobi, so that's why it was not parametrised with a "solver" argument", it does not make sense to pass a different solver to this test. I already did the work to support filtering solver-dependant tests that don't have a "solver" argument, so I would say to just leave the existing tests as is. I don't quite remember why I even proposed the second option.

[OrestisLomis]

Well yes, the test is also renamed to be general in #950. I don't think it makes sense to make a new class for each solver, instead of just adding the solver to the list of required solvers. I think this also makes more sense in general for other tests in the future.

[OrestisLomis]

I don't think this really works actually. If I put an assert False in the first line of CPM_Exact.mus_native() then the test suite does not actually catch this. So the tests are still not run. Even if I put solver as an argument in the class. Please have another look when you have some time.

[ThomSerg]

I now added support for solver dependent tests that aren't parametrised. When you have a test that is really specific to only one solver, it's a bit dubious to then be required to parametrise it with argument "solver" to be able to use the marker requires_solver. Should fix your problem @OrestisLomis?