Improve xfails by allowing specifying expected errors | test(torchlib)

[justinchuby]

• Allow specifying expected errors in xfails
• Raise RuntimeError instead of AssertionError when ORT raises an error in test for more accurate xfails.
• Specify the expected errors in some xfails (as a first pass)

Fixes #794

[BowenBao]

Thanks @justinchuby this is great! Can we guard on the error message too? Exception type feels a bit broad, for example I may want to flag change in behavior when ORT starts to complain about mismatching type instead of type not implemented.

[justinchuby]

The error message is harder because there isn't a pytest decorator for it. I could implement one but that will take longer. Do you have any suggestions? Thanks!

[BowenBao]

For my own understanding, why do we seem to decorate xfail at two places? One here, and the other down at

def normal_xfail_skip_test_behaviors(
    ...
        if test_behavior == "xfail":
            pytest.xfail(reason=reason)

[justinchuby]

There is the decorator that will decorate the test method, and there are context managers that handle the subtests. I haven't found a good way to unify the two. I may be possible for us to create our own decorators but I am not sure how it is supported by pytest?

[titaiwangms]

It's not supported..

[BowenBao]

Need to set strict = True?

https://docs.pytest.org/en/7.3.x/reference/reference.html#pytest-mark-xfail-ref

[justinchuby]

When strict=True any passing subtests will fail the whole test method. We have cases where some subtests fail and we may still want to run the test. Any suggestions?

[BowenBao]

Passing in the unittest instance, can we do

        if test_behavior == "xfail":
            if raises is not None:
                with test_case.assertRaisesRegex(raises, expected_error_message_regex):
                    raise
            pytest.xfail(reason=reason)

[BowenBao]

nvm, I think we need a context manager similar to this but wrapped over subtest calling.

edit: wait.. this function is already that context manager, so it should work?

[justinchuby]

It should work for any subtests, but I am not sure how to support the test methods?

[titaiwangms]

The error message is harder because there isn't a pytest decorator for it. I could implement one but that will take longer. Do you have any suggestions? Thanks!

A naive way would be we do string match on reason and exception message in normal_xfail_skip_test_behaviors, and it would raise xfail if only they match at some points.

I guess it doesn't have to be strict. Just point out the error occurred location and type.

[justinchuby]

The error message is harder because there isn't a pytest decorator for it. I could implement one but that will take longer. Do you have any suggestions? Thanks!

A naive way would be we do string match on reason and exception message in normal_xfail_skip_test_behaviors, and it would raise xfail if only they match at some points.

I guess it doesn't have to be strict. Just point out the error occurred location and type.

That's for the subtest I think? I am thinking if there's a way to use the decorators

[titaiwangms]

The error message is harder because there isn't a pytest decorator for it. I could implement one but that will take longer. Do you have any suggestions? Thanks!

A naive way would be we do string match on reason and exception message in normal_xfail_skip_test_behaviors, and it would raise xfail if only they match at some points.
I guess it doesn't have to be strict. Just point out the error occurred location and type.

That's for the subtest I think? I am thinking if there's a way to use the decorators

Do we have more cases needing xfail on whole test? The other dtypes?

[justinchuby]

Yes. Sometimes ORT fails for the op so we xfail the whole test