This is an archive of the discontinued LLVM Phabricator instance.

Add SKIPPED to known result names.
ClosedPublic

Authored by nigelp-xmos on Jun 18 2021, 3:53 AM.

Details

Summary

A new test result on the buildbot workers, "SKIPPED", was introduced by D102754.

This was not specifically handled in LitTestCommand.py, so some build results displayed, "Unexpected test result output SKIPPED", without the number of tests producing that result.

This patch is to handle "SKIPPED" and display the number of tests producing that result, as "X skipped unit tests".

The patch will not turn a red build into a green one, just display the relevant number of tests.

I don't have a local copy of the buildbot infrastructure to test this for side effects, but @gkistanova staged the change 2 months ago (June 2021), and it is displaying the results as intended.

The first build of interest, showing "Unexpected test result output SKIPPED", was:
https://lab.llvm.org/staging/#/builders/145/builds/996

A recent build showing the intended "X skipped unit tests" is:
https://lab.llvm.org/staging/#/builders/145/builds/2517

Event Timeline

nigelp-xmos created this revision.Jun 18 2021, 3:53 AM
nigelp-xmos requested review of this revision.Jun 18 2021, 3:53 AM

I have staged this patch.

The both mentioned builders are still red, and at a quick glance the failures are not related to the SKIPPED handling.

Could you point to the exact builds or revisions which could be used for the reproduction the issue and for the check of the fix, please?

Thanks @gkistanova .

The clang-xcore build status changed from "Unexpected test result output SKIPPED" in https://lab.llvm.org/staging/#/builders/145/builds/1340, to "X skipped unit tests" in https://lab.llvm.org/staging/#/builders/145/builds/1342. If that was when the patch was staged, it has worked on the display of results.

But I don't have the buildmaster so I am not sure if the describe() function result, with the amended text, is used apart from the display.

The first clang-xcore build showing "Unexpected test result output SKIPPED" was https://lab.llvm.org/staging/#/builders/145/builds/996.

Yes, red status looks as expected. The clang-xcore build has several known failures that we are working through. I don't know about clang-x86-ninja-win10; I just cited it because I saw it also encountered the SKIPPED issue; but it too was red before this change, and before SKIPPED appeared. They weren't red because of SKIPPED. I was just aiming to replace the text "unexpected test result" with the count against "skipped".

nigelp-xmos edited the summary of this revision. (Show Details)Sep 13 2021, 1:47 AM
nigelp-xmos added a reviewer: ikudrin.

Thanks. The change looks reasonable to me.

Comparing similar builders in the LLVM Buildbot and LLVM Staging sites (e.g. Buildbot/clang-armv7-quick/3395 vs Staging/clang-armv7-quick/157), the report of the latter looks definitely better. Still, the final approval should come from the build bot folks.

@ikudrin Thanks for the comment and the buildbot links. Yes, I won't check in till approved by buildbot maintainers.

gkistanova accepted this revision.Sep 13 2021, 9:04 AM

Thanks, Nigel!
LGTM with a nitpick.

How about removing the word “unit” from the description? This will be in line with the UNSUPPORTED description and be shorter without loosing information.

This revision is now accepted and ready to land.Sep 13 2021, 9:04 AM

Thanks @gkistanova . Yes, I will remove "unit" as suggested.

nigelp-xmos edited the summary of this revision. (Show Details)

Remove "unit" from description as recommended in review.

This revision was automatically updated to reflect the committed changes.