How to properly test a patch

I just had a push reverted because linking failed with shared libraries. I, unsurprisingly, had been testing on my local machine with static libraries.

I’ve heard mention that someone was trying to create a buildbot that could be used for testing. Perhaps I should have paid more attention to this thread, but perhaps some kind soul could fill me in on testing.

Thanks,

Craig

Hi,

It is really hard to cover the entire test matrix, and be bullet-proof: next time you may get a revert because a sanitizer bot will fail (ASAN/UBSAN/TSAN), or because you broke the IBM SystemZ bot or something like that.

There is some testing on Phabricator revisions, but I believe it only builds a single config on Linux and Windows.

Some folks are looking into expanding the pre-merge testing flow, but it’ll take time!

Craig,

There is quite a few Flang BuildBot workers available here: Buildbot
(type "flang" in the search box to filter the other ones out). Quite a few configurations are tested, but as Mehdi points out, these workers run post-merge.

I'm not sure how to check the available configurations for pre-merge testing. I do know that there are both Linux and Windows bots running for every patch that you submit to Phabricator. A summary is usually available in the "Details" box at the top (alongside "Reviewers" etc).

Most of us have had a patch reverted at some point - it's just really hard to cover the whole matrix. Your patch (⚙ D108059 [flang] Make 'this_image()' an intrinsic function) landed with:
> This revision was landed with ongoing or failed builds.
Perhaps pre-merge testing did hint something? I'm not sure whether it's possible to dig out the actual log, so we'll probably never know. And false negatives do happen occasionally.

I'm not aware of any efforts to improve the upstream testing of LLVM Flang just now, but every now and then some extra configuration is added. Recently, Michael Kruse added a Windows post-merge worker. That's been a huge improvement for us.

Btw, most of the time, "shared library builds" are a better at capturing dependency/build errors and that's what I tend to use by default.

-Andrzej

Craig,

There is quite a few Flang BuildBot workers available here:
https://lab.llvm.org/buildbot/#/builders
(type “flang” in the search box to filter the other ones out). Quite a
few configurations are tested, but as Mehdi points out, these workers
run post-merge.

I’m not sure how to check the available configurations for pre-merge
testing. I do know that there are both Linux and Windows bots running
for every patch that you submit to Phabricator. A summary is usually
available in the “Details” box at the top (alongside “Reviewers” etc).

Most of us have had a patch reverted at some point - it’s just really
hard to cover the whole matrix. Your patch
(https://reviews.llvm.org/D108059) landed with:

This revision was landed with ongoing or failed builds.
Perhaps pre-merge testing did hint something? I’m not sure whether it’s
possible to dig out the actual log, so we’ll probably never know. And
false negatives do happen occasionally.

The log is still accessible, in the comment feed you can look for the most recent Harbormaster:

In this case it only found a clang-tidy failure.

I’m not aware of any efforts to improve the upstream testing of LLVM
Flang just now, but every now and then some extra configuration is
added. Recently, Michael Kruse added a Windows post-merge worker. That’s
been a huge improvement for us.

Btw, most of the time, “shared library builds” are a better at capturing
dependency/build errors and that’s what I tend to use by default.

+1: the shared lib config is more sensitive to missing dependencies in CMake.