TST: Only skip/xfail tests whose ids exactly match

[lithomas1]

No description provided.

[honno]

Thanks for the PR but its intentional behaviour for skip/xfail'd tests to match multiple test cases, which is mostly useful for parametrized tests where adopters know they'll all fail.

[lithomas1]

Thanks for the PR but its intentional behaviour for skip/xfail'd tests to match multiple test cases, which is mostly useful for parametrized tests where adopters know they'll all fail.

The problem, though is that it becomes impossible to skip some test cases without skipping other test cases that might not fail.

(e.g. skipping array_api_tests/test_array_object.py::test_setitem would also skip array_api_tests/test_array_object.py::test_setitem_masking )

[asmeurer]

I asked @lithomas1 to open this PR.

Can we make it so this either matches the full test with the parameterization, or a full test name without the parameterization part? Verboseness is not an issue for a file, but specificity is.

[honno]

The problem, though is that it becomes impossible to skip some test cases without skipping other test cases that might not fail.

Ah I see.

Can we make it so this either matches the full test with the parameterization, or a full test name without the parameterization part? Verboseness is not an issue for a file, but specificity is.

I'll mull over but if you folks have an idea please update the PR. I don't think it's a good idea accepting this PR as-is because 1) it makes specifying parameterised tests very verbose 2) breaks existing configs without any warnings/etc.. I imagine some regex for pytest's square bracket syntax for atomic "parameter" test cases is what we'd want to look at, which would avoid the need to break existing configs too.

[asmeurer]

Just to be sure, there are some warnings that are printed when an xfail doesn't match anything.

[lithomas1]

I'll mull over but if you folks have an idea please update the PR. I don't think it's a good idea accepting this PR as-is because 1) it makes specifying parameterised tests very verbose 2) breaks existing configs without any warnings/etc.. I imagine some regex for pytest's square bracket syntax for atomic "parameter" test cases is what we'd want to look at, which would avoid the need to break existing configs too.

Maybe we can turn the lines of the skip file into a regex?

I think with a regex I can use $ to "end" the id I think. This works around my problem I think.

[lithomas1]

I technically don't need this PR anymore (since I resolved the failures mentioned above), but this might still be a good idea regardless.

(e.g. I'm pretty sure this also happens for torch in array-api-compat
https://github.com/data-apis/array-api-compat/actions/runs/7590566384/job/20677289445#step:6:1588, where test_setitem is xfailed but test_setitem_masking xpasses)

[asmeurer]

Making it a regex would be even more breaking than this. The [] in any current pattern would be treated as a character class in a regex, and so all skip files would need to be updated. We could add something like a regex: prefix to make a line match as a regex.

However, I'm currently not convinced that this change is an issue. Are there xfails files out there that are making use of this prefix logic?

[asmeurer]

Do we have reason to believe that it isn't enough to only accept

• a test file (like array_api_tests/test_special_cases.py),
• a test name (like array_api_tests/test_special_cases.py::test_binary), or
• a full test with parameterization (like array_api_tests/test_special_cases.py::test_binary[__floordiv__(x1_i is +infinity and isfinite(x2_i) and x2_i > 0) -> +infinity])?

I don't see the value in accepting subsets other than those three cases, and trying to do so would risk lead to false positives like the test_setitem one.

The only thing I can think of is you might want to skip all parameterized tests for a given function like array_api_tests/test_special_cases.py::test_binary[__floordiv__], because maybe you know __floordiv__ can never work.

Another option would be to only match those three cases if the skip name starts with array_api_tests/, and do in matching only when it doesn't.

[honno]

Do we have reason to believe that it isn't enough to only accept

• a test file (like array_api_tests/test_special_cases.py),
• a test name (like array_api_tests/test_special_cases.py::test_binary), or
• a full test with parameterization (like array_api_tests/test_special_cases.py::test_binary[__floordiv__(x1_i is +infinity and isfinite(x2_i) and x2_i > 0) -> +infinity])?

I don't see the value in accepting subsets other than those three cases, and trying to do so would risk lead to false positives like the test_setitem one.

I don't have a sense of usage actually tbf, so maybe no one's currently skipping any of the parametrized tests. I am generally worried that for things like the special case tests, if you want to skip/xfail a whole test case, you'd have to manually specify 100+ cases.

I think with a regex I can use $ to "end" the id I think. This works around my problem I think.

Making it a regex would be even more breaking than this. The [] in any current pattern would be treated as a character class in a regex, and so all skip files would need to be updated. We could add something like a regex: prefix to make a line match as a regex.

I agree regex for things like this seems a bit too much esp. considering how square brackets, but it is a well-defined and well-known format in the end if we did want to skip parameterized tests (or even specifically a subset of parameters) 🤔

[asmeurer]

Maybe we could also extend it to allow skipping test_binary[__floordiv__] or something like that. Or we could rewrite the tests so that __floordiv__ is part of the test name.

[hameerabbasi]

We ran into the exact case of test_getitem and test_getitem_masking

Do we have reason to believe that it isn't enough to only accept

* a test file (like `array_api_tests/test_special_cases.py`),

* a test name (like `array_api_tests/test_special_cases.py::test_binary`), or

* a full test with parameterization (like `array_api_tests/test_special_cases.py::test_binary[__floordiv__(x1_i is +infinity and isfinite(x2_i) and x2_i > 0) -> +infinity]`)?

I'd also accept a directory/partial path, in case we split into directories later, but otherwise the list seems complete to me.

[hameerabbasi]

I've opened #273 with my suggestions.

[asmeurer]

This was superceeded by #273