Page MenuHomePhabricator

Fix undeterministic call to shouldPoll() in Avalanche tests
ClosedPublic

Authored by jasonbcox on Feb 6 2019, 00:00.

Details

Summary

These tests pass consistently with make check but fail ninja check consistently with the following setup:

gcc 8.1.0
clang 7.0.1
boost 1.58.0

The failure occurs at step i = 9, where registerPoll() is called, setting inflight to 10
and then immediately after, the call to shouldPoll() returns false.

Based on reading of the test, it seems clear that shouldPoll() should be evaluated before registerPoll() is called, but apparently compiler differences evaluate the BOOST_CHECK_EQUAL macro differently.

Before writing this diff, I attempted to write the test in such a way as to evaluate registerPoll() and shouldPoll(), in that order, in intermediate variables, as this is the expected order of evaluation when reading the BOOST_CHECK_EQUAL call. This test fails for ninja and make as expected:

-        BOOST_CHECK_EQUAL(vrinflight.registerPoll(), vrinflight.shouldPoll());
+        bool rp = vrinflight.registerPoll();
+        bool sp = vrinflight.shouldPoll();
+        BOOST_CHECK_EQUAL(rp, sp);
Test Plan
make check
ninja check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jasonbcox created this revision.Feb 6 2019, 00:00
Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 6 2019, 00:00
Herald added a subscriber: schancel. · View Herald Transcript
deadalnix requested changes to this revision.Feb 6 2019, 15:15
This revision now requires changes to proceed.Feb 6 2019, 15:15
jasonbcox edited the summary of this revision. (Show Details)Feb 6 2019, 17:17
jasonbcox edited the summary of this revision. (Show Details)Feb 6 2019, 17:20
jasonbcox edited the summary of this revision. (Show Details)
deadalnix accepted this revision.Feb 6 2019, 17:22
This revision is now accepted and ready to land.Feb 6 2019, 17:22
This revision was automatically updated to reflect the committed changes.