I’m looking for input on how we can protect contributors from getting misleading ‘fail’ status on their PRs, when the ‘failure’ is totally unrelated to their PR contribution.
Here is an example of the most recent PR, that happened to be 0.7.x-related PR https://github.com/fastai/fastai/pull/900. It reports failure https://dev.azure.com/fastdotai/fastai/_build/results?buildId=216&view=logs because one of the non-deterministic tests has failed again. But it’s the same issue with any PRs 1.0.x or 0.7-alike.
As we discussed, those tests will fail occasionally due to their nature, and while we try to keep master as correct as possible, it’s OK if CI builds occasionally fail. But it’s not OK, IMHO, if PR builds fail when it’s not contributor’s error. Especially in cases like the one I present where the error doesn’t even have anything to do with the proposed contribution (0.7.x patch).
Thoughts?
The failure was:
> assert abs(x.mean()) < abs(m)
E assert tensor(0.1254) < tensor(0.1231)
E + where tensor(0.1254) = abs(tensor(-0.1254))
E + where tensor(-0.1254) = <built-in method mean of Tensor object at 0x1237f2090>()
E + where <built-in method mean of Tensor object at 0x1237f2090> = tensor([[[[-0.4745, -0.4745, -0.4745, ..., -0.4745, -0.4745, -0.4745],\n [-0.4745, -0.4745, -0.4745, ..., -0..., -0.4745, ..., -0.4745, -0.4745, -0.4745],\n [-0.4745, -0.4745, -0.4745, ..., -0.4745, -0.4745, -0.4745]]]]).mean
E + and tensor(0.1231) = abs(tensor(0.1231))
tests/test_vision.py:36: AssertionError
but please let’s focus here not on trying to improve the test, since it’ll never be 100% failure-proof, but on how to make contributors’ experience better.
Thank you.
And my idea about solving this is to have a special flag or env var passed to pytest that when used, it will not fail such tests but xfail instead.
And if we are ok with occasional CI build failures, but not OK with PR build failures, we could have separate scripts for CI builds vs PR builds, except it’d be a lot of duplication. Perhaps there is a way to tell from the environment whether this is a CI or PR build, and only pass such flag in PR builds.