Enhance test deduplication in PyTorch test parsing - #4203
Conversation
Some tests have the full filename in their class name. This is partially accounted for in the `suite_name == 'run_test'` branch where the suite name is removed from the classname prefixes. However, this is not enough. Depending on how the test is (re)run "regular" tests also have a prefix. E.g. from `python-pytest/test_nestedtensor/test_nestedtensor-*.xml` ``` <testcase classname="TestNestedTensorOpInfoCPU" name="test_nested_tensor_non_contiguous_mutation_cpu" time="4.2" file="test_nestedtensor.py"/> <testcase classname="test.test_nestedtensor.TestNestedTensorOpInfoCPU" name="test_nested_tensor_non_contiguous_mutation_cpu" time="4.2"/> ``` Both refer to the same test and deduplication is essential in case one of those passed. A common pattern is, that the short classnames (i.e. where only a classname is present, not a path-like) have the `file` attribute set. For simplicity and to not rely on this quirk I moved the suite-prefix-stripping to all kinds of test results and also remove the last part of the suite which was previously not yet excluded. I.e. it could keep `test_nestedtensor.TestNestedTensorOpInfoCPU`. Seems that is not necessary and the class name alone should be enough.
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Aggregated manually |
When a XML entry considers a test as skipped it should be discarded if there is an other entry, e.g. a success or failure which is more important. However for tests marked as XFAIL (expected failure) the "success" is a still marked as a skip but with an xfail type due to limitations of the format. Hence it should be treated as success.
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (total: 11 hours 49 mins 25 secs) (2 easyconfigs in total) |
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (total: 20 hours 1 min 56 secs) (2 easyconfigs in total) |
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (total: 20 hours 28 mins 9 secs) (2 easyconfigs in total) |
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (total: 26 hours 21 mins 51 secs) (2 easyconfigs in total) |
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 2 (total: 32 hours 1 min 32 secs) (2 easyconfigs in total) |
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 3 (total: 69 hours 20 mins 4 secs) (3 easyconfigs in total) |
|
@Flamefire Can you look into the failed test reports? Not sure they should be blocking the PR... |
|
I'll look into one failure but they are all unrelated to this change here. I added comments for each |
Some tests have the full filename in their class name. This is partially accounted for in the
suite_name == 'run_test'branch where the suite name is removed from the classname prefixes.However, this is not enough. Depending on how the test is (re)run "regular" tests also have a prefix. E.g. from
python-pytest/test_nestedtensor/test_nestedtensor-*.xmlBoth refer to the same test and deduplication is essential in case one of those passed.
A common pattern is, that the short classnames (i.e. where only a classname is present, not a path-like) have the
fileattribute set.For simplicity and to not rely on this quirk I moved the suite-prefix-stripping to all kinds of test results and also remove the last part of the suite which was previously not yet excluded. I.e. it could keep
test_nestedtensor.TestNestedTensorOpInfoCPU. Seems that is not necessary and the class name alone should be enough.This avoids failures like:
The actual (reduced) sample that caused this failure is included in the tests now
The mentioned change is reflected in the test:
Out of this:
The common prefix
test.dist.foo.baris treated as the suite name and now fully removed instead of keeping the last element.