<div dir="ltr">Hi,<div><br></div><div>Yes, this test hardness has been broken for years. I discovered that and fixed it in January (<a href="https://gerrit.gromacs.org/#/c/7478/">https://gerrit.gromacs.org/#/c/7478/</a>), 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!</div><div><br></div><div>There is a long standing open Redmine to replace it, and I have been working on the replacement for years, e.g. <a href="https://gerrit.gromacs.org/#/c/7736/">https://gerrit.gromacs.org/#/c/7736/</a> 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.</div><div><br></div><div>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.</div><div><br></div><div>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!</div><div><br></div><div>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.</div><div><br></div><div>Mark</div></div><br><div class="gmail_quote"><div dir="ltr">On Sat, Jun 9, 2018 at 4:30 PM Szilárd Páll <<a href="mailto:pall.szilard@gmail.com">pall.szilard@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
I know this issue has been noted in the past, but I am not sure if<br>
this has been addressed.<br>
<br>
The following sequence of commands illustrates that in my relaese-2018<br>
git tip of the branch build the ctest execution of the regressiontests<br>
claims pass, but a number of tests do fail and gmxtests seems to<br>
return 0. Thoughts?<br>
<br>
<br>
$ cmake ../ -; make check -j8<br>
[...]<br>
33/39 Test #33: MdrunMpiTests .................... Passed 3.36 sec<br>
Start 34: regressiontests/simple<br>
34/39 Test #34: regressiontests/simple ........... Passed 6.70 sec<br>
Start 35: regressiontests/complex<br>
35/39 Test #35: regressiontests/complex .......... Passed 179.95 sec<br>
Start 36: regressiontests/kernel<br>
36/39 Test #36: regressiontests/kernel ........... Passed 65.51 sec<br>
Start 37: regressiontests/freeenergy<br>
37/39 Test #37: regressiontests/freeenergy ....... Passed 13.12 sec<br>
Start 38: regressiontests/pdb2gmx<br>
38/39 Test #38: regressiontests/pdb2gmx .......... Passed 26.82 sec<br>
Start 39: regressiontests/rotation<br>
39/39 Test #39: regressiontests/rotation ......... Passed 5.81 sec<br>
[...]<br>
<br>
$ PATH=$gmxdir:$PATH perl <a href="http://gmxtest.pl" rel="noreferrer" target="_blank">gmxtest.pl</a> -xml complex -ntomp 8 -npme 0 -nt<br>
1; echo -e "\n\n===> RETVAL: $? <==="<br>
[...]<br>
11 out of 51 complex tests FAILED<br>
<br>
<br>
===> RETVAL: 0 <===<br>
<br>
<br>
--<br>
Szilárd<br>
-- <br>
Gromacs Developers mailing list<br>
<br>
* Please search the archive at <a href="http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List" rel="noreferrer" target="_blank">http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List</a> before posting!<br>
<br>
* Can't post? Read <a href="http://www.gromacs.org/Support/Mailing_Lists" rel="noreferrer" target="_blank">http://www.gromacs.org/Support/Mailing_Lists</a><br>
<br>
* For (un)subscribe requests visit<br>
<a href="https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers" rel="noreferrer" target="_blank">https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers</a> or send a mail to <a href="mailto:gmx-developers-request@gromacs.org" target="_blank">gmx-developers-request@gromacs.org</a>.</blockquote></div>