Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a VirtualClock to make tests more efficient and reliable #389

Closed
AndrejMitrovic opened this issue Oct 18, 2019 · 3 comments
Closed
Assignees
Labels
C.General An issue which doesn't fit in the other categories (not blockchain related) Story-Points:3 This takes 1 to 3 days to complete type-enhancement An improvement of existing functionalities
Milestone

Comments

@AndrejMitrovic
Copy link
Contributor

We already use something similar for our BanManager tests, where we override the BanManager's clock routine to return a "fake" clock time. Stellar-core does something similar for its entire test-suite, and especially for the SCP tests. With a virtual clock we can make tests more efficient, and we can simulate arbitrary delays without having to physically wait for the wall clock time to advance.

@AndrejMitrovic AndrejMitrovic added the type-enhancement An improvement of existing functionalities label Oct 18, 2019
@AndrejMitrovic AndrejMitrovic self-assigned this Oct 18, 2019
@AndrejMitrovic AndrejMitrovic removed their assignment Nov 13, 2019
@AndrejMitrovic AndrejMitrovic self-assigned this Dec 2, 2019
@AndrejMitrovic AndrejMitrovic added the Story-Points:2 This takes 1 day to complete label Dec 2, 2019
@bpalaggi bpalaggi added this to the 2. Validator milestone Dec 2, 2019
@bpalaggi bpalaggi removed the Story-Points:2 This takes 1 day to complete label Jan 7, 2020
@Geod24
Copy link
Collaborator

Geod24 commented Jan 22, 2020

Is this becoming more of a priority now that we have SCPD ?

@AndrejMitrovic
Copy link
Contributor Author

AndrejMitrovic commented Feb 3, 2020

Is this becoming more of a priority now that we have SCPD ?

In short: yes, for testing failures. We don't have those tests yet in Agora, but I plan to add them.

Here's some info for the whole team:

So timer's in SCP are only used to track communication failures within a given time-frame. SCP basically does this (pseudocode):

// See Slot.d : timerIDs
startTimer(BALLOT_PROTOCOL_TIMER, &onFailure, 1.seconds * attempt_number);  

... // actually attempt this ballot, this might take some time

// kills the timer before it has the chance to fire
stopTimer(BALLOT_PROTOCOL_TIMER);  

Where onFailure() would really just restart the whole process but also increment attempt_number to gradually increase the timeout. In SCP parlance, there can be multiple rounds of voting for a given slot (block height), these rounds are the Ballots (BallotProtocol). In our yellow paper the "voting round" means a completed externalized block (which may consist of multiple ballots / multiple voting rounds).

@AndrejMitrovic AndrejMitrovic removed their assignment Mar 10, 2020
AndrejMitrovic added a commit to AndrejMitrovic/agora that referenced this issue May 20, 2020
We did not account for the fact that our unittests
are built with code coverage support.

For each line in the D code, the compiler injects
an increment opcode do a compiler-built bit array.

This can significantly slow down processing,
so as a counter-measure we can increase the
timeouts to make the test-suite less brittle.

The ultimate solution would be to replace the use
of real time with virtual time. See bosagora#389.
AndrejMitrovic added a commit to AndrejMitrovic/agora that referenced this issue May 20, 2020
We did not account for the fact that our unittests
are built with code coverage support.

For each line in the D code, the compiler injects
an increment opcode do a compiler-built bit array.

This can significantly slow down processing,
so as a counter-measure we can increase the
timeouts to make the test-suite less brittle.

The ultimate solution would be to replace the use
of real time with virtual time. See bosagora#389.
AndrejMitrovic added a commit to AndrejMitrovic/agora that referenced this issue May 20, 2020
We did not account for the fact that our unittests
are built with code coverage support.

For each line in the D code, the compiler injects
an increment opcode do a compiler-built bit array.

This can significantly slow down processing,
so as a counter-measure we can increase the
timeouts to make the test-suite less brittle.

The ultimate solution would be to replace the use
of real time with virtual time. See bosagora#389.
Geod24 pushed a commit that referenced this issue May 20, 2020
We did not account for the fact that our unittests
are built with code coverage support.

For each line in the D code, the compiler injects
an increment opcode do a compiler-built bit array.

This can significantly slow down processing,
so as a counter-measure we can increase the
timeouts to make the test-suite less brittle.

The ultimate solution would be to replace the use
of real time with virtual time. See #389.
@Geod24 Geod24 added the C.General An issue which doesn't fit in the other categories (not blockchain related) label Jul 5, 2020
@bpalaggi bpalaggi added the Story-Points:3 This takes 1 to 3 days to complete label Aug 25, 2020
@AndrejMitrovic
Copy link
Contributor Author

Resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C.General An issue which doesn't fit in the other categories (not blockchain related) Story-Points:3 This takes 1 to 3 days to complete type-enhancement An improvement of existing functionalities
Projects
None yet
Development

No branches or pull requests

3 participants