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

[Szilárd Páll]

On Sat, Jun 9, 2018 at 6:36 PM, Mark Abraham <mark.j.abraham at gmail.com> wrote:
> 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.

Thanks. I asked because I spent some time looking in the
regressiontest repo, redmine, emails, and could not find a trace of
the issue, but I assumed I'd find at least a redmine on it targetted
for a release so we know we need to deal with it and it's not just an
issue that's lost in the sea of WIP on gerrit.

I saw the issue has been merged now, but I can't find a redmine issue
or release note updates. Given that this can lead to different result
after the update, i.e. failing "make check" after the update (in known
cases that I've observed which prompted my email), we should probably
note something, shouldn't we?

> 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!

This and the rest is relevant, but it's largely off topic ("topics"
actually) so I'll leave that for separate discussions.

>
> 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.
>
>
> --
> 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.