[gmx-developers] r2018 regressiontest script return value does not reflect test results

[Mark Abraham]

Hi,

Yes, this test hardness has been broken for years. I discovered that and
fixed it in January (https://gerrit.gromacs.org/#/c/7478/), but nobody has
voted plus 2 on it yet, mostly because few people know Perl or how the
harness works. We also have large gaps in our test coverage (e.g. we are
fixing bugs after refactoring the leapfrog update code, because we have no
testing with freeze groups) which we must remember is a comparable or
larger problem than whether we cover all SIMD flavours or compiler versions
in pre-submit configurations. This is something that we must all work
together to improve!

There is a long standing open Redmine to replace it, and I have been
working on the replacement for years, e.g.
https://gerrit.gromacs.org/#/c/7736/ and parent, but people often say they
have too little experience of GoogleTest, too. So, we must be tougher on
each other in review, so that we acquire that experience :-) Absolutely we
should have had unit tests on those update kernels - Berk's improvements to
them make it reasonably easy to do now.

At the same time, we need to remember that changes in review don't need to
be perfect - we don't gain much from fixing trivial spelling errors in
newly written documentation, nor should we wait for two-person review of
patches that replace C-style snew with C++ std::vector. If someone has said
they want to review a patch but just don't have time this week, then that's
OK, but neither can they drag things on forever. :-) If a gerrit patch
makes a single logical change, and e.g. links a Redmine with a sound plan
that has no dissent, passes Jenkins and doesn't make something materially
worse, then it should not get held up on trivia. If it has a +2 from a
developer with suitably experience of that area, then I suggest that it
should just go in after a week for others to see and comment. If a patch
might break some platform, or simulation type, or affect performance then
we want more scrutiny.

This approach requires that developers stay active scanning Gerrit for
changes that affect their projects, past and current. I do not think we
should operate in a mode where developers have to write each other email to
ask for a review - we all get more than enough email!

Also we should bless some newer people with the gmx-core privilege of
voting +2. As always, reviewers must use their judgment and vote suitably
on patches that align with their experience - in particular I propose Paul
Bauer, Pascal Merz and Eric Irrgang for that privilege.

Mark

On Sat, Jun 9, 2018 at 4:30 PM Szilárd Páll <pall.szilard at gmail.com> wrote:

> Hi,
>
> I know this issue has been noted in the past, but I am not sure if
> this has been addressed.
>
> The following sequence of commands illustrates that in my relaese-2018
> git tip of the branch build the ctest execution of the regressiontests
> claims pass, but a number of tests do fail and gmxtests seems to
> return 0. Thoughts?
>
>
> $ cmake ../ -; make check -j8
> [...]
> 33/39 Test #33: MdrunMpiTests ....................   Passed    3.36 sec
>       Start 34: regressiontests/simple
> 34/39 Test #34: regressiontests/simple ...........   Passed    6.70 sec
>       Start 35: regressiontests/complex
> 35/39 Test #35: regressiontests/complex ..........   Passed  179.95 sec
>       Start 36: regressiontests/kernel
> 36/39 Test #36: regressiontests/kernel ...........   Passed   65.51 sec
>       Start 37: regressiontests/freeenergy
> 37/39 Test #37: regressiontests/freeenergy .......   Passed   13.12 sec
>       Start 38: regressiontests/pdb2gmx
> 38/39 Test #38: regressiontests/pdb2gmx ..........   Passed   26.82 sec
>       Start 39: regressiontests/rotation
> 39/39 Test #39: regressiontests/rotation .........   Passed    5.81 sec
> [...]
>
> $ PATH=$gmxdir:$PATH perl gmxtest.pl -xml complex -ntomp 8 -npme 0 -nt
> 1; echo -e "\n\n===> RETVAL: $? <==="
> [...]
> 11 out of 51 complex tests FAILED
>
>
> ===> RETVAL: 0 <===
>
>
> --
> Szilárd
> --
> Gromacs Developers mailing list
>
> * Please search the archive at
> http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List before
> posting!
>
> * Can't post? Read http://www.gromacs.org/Support/Mailing_Lists
>
> * For (un)subscribe requests visit
> https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers
> or send a mail to gmx-developers-request at gromacs.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://maillist.sys.kth.se/pipermail/gromacs.org_gmx-developers/attachments/20180609/57872f40/attachment.html>