Test Native MUS function

[OrestisLomis]

Not sure if this is the correct way to do things, but apparently the native_mus tests are never run according to Henk's code cov and I confirm this by just putting an assert False in the function. Since the test is for exact only I wrote @pytest.mark.requires_solver("exact") above the testclass, which probably is related to the issue.

@ThomSerg do you know more about this?

[hbierlee]

As discussed offline, it's indeed the missing solver arg from the test functions. For me it makes sense to just add that argument to all the MUS tests, as they have now become solver-specific.

[OrestisLomis]

The mus functions aren't solver specific so I do not think that is the best solution; instead I only added the solver argument to the TestNative, which after some trial and error seems to be working now

[hbierlee]

Well, maybe we need a third opinion, but I think that the other MUS tests might as well be good to parametrize for all solvers. They do have solver arguments, so we're hoping that any CPMpy solver would be able to compute the MUS, so I think this is a good opportunity to test that assumption. It would also simplify the TestNativeMus to not have the exceptional autouse decorator, but just requires_solvers meaning: unlike the other MUS algorithms, this one requires exact.

[OrestisLomis]

I'm not sure I agree with that suggestion, but either way it is something for a different PR then. Because while the solver argument exists in the MUS function it still only accepts solvers which allow solving under assumptions. Besides the goal of the tests is to tests the MUS algorithms which are agnostic of the solver (or solving technology in general) and hence I don't see why we would duplicate these tests for all solvers.

[hbierlee]

Ok, but maybe the best solution is to then still parametrize the tests, but add requires_solver("ortools") to TestMUS, and requires_solver("exact") to the TestNativeMUS? That's just the regular way to indicate which solver should run which tests, and my preference over the autouse addition.

Then we can easily add a few more (assumption-based) solvers to the TestMUS if we wish (in principal I agree its duplication because we're testing the MUS code, so but in practice the assumptions are thinly tested test_solverinterface; fine to have a separate mini PR for that)

If you still do not agree, then we need a third opinion

[tias]

I don't like the autouse much either, would prefer something more explicit.

For Thomas to comment on, as he knows the parameterization best (can also be at the next dev meeting)

[OrestisLomis]

I do like Henk's final suggestion, also sounds like something that can be added in this PR in the end. I'm just not sure exactly how to do it without the autouse to actually have the solver available in the setup (where the mus_func is defined). Summoning @ThomSerg again.

[hbierlee]

Doesn't seem easy to add the solver fixture to setup_class, so I think the easiest way around that is if you make self.mus_func = mus and pass the solver in the test methods themselves (some duplication but I think avoiding it is not worth it)