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

Move test timeouts to constants. #367

Merged
merged 2 commits into from Dec 8, 2016
Merged

Move test timeouts to constants. #367

merged 2 commits into from Dec 8, 2016

Conversation

no2chem
Copy link
Member

@no2chem no2chem commented Dec 8, 2016

This PR moves timeouts to constants, so that in the future different timeouts can be
set in Travis. It also forces the user to select from a set of four timeout lengths.

…future, this will be configured differently on Travis
@Maithem
Copy link
Contributor

Maithem commented Dec 8, 2016

Timeouts are used all over the place, not just with the scheduled executor, but this is a good start. Other timeouts can be discovered and fixed as we go

@slfritchie
Copy link
Contributor

Michael, this patch does something weird with changing the typing of CancellationException. TravisCI (and my run of the suite locally) shows 18 errors, starting with https://travis-ci.org/CorfuDB/CorfuDB/builds/182238645#L1686

@no2chem
Copy link
Member Author

no2chem commented Dec 8, 2016

@slfritchie It's not changing the typing of CancellationException. What makes you think that? The patch currently has a bug because it uses getNanos() instead of toNanos(), resulting in a timeout that is 0s, so the tests are cancelled immediately. Fixing soon.

@slfritchie
Copy link
Contributor

Good to know about the units problem.

My question about typing came from this test output:

<<< FAILURE! - in org.corfudb.AbstractCorfuConcurrencyTest
concurrentTestsThrowExceptions(org.corfudb.AbstractCorfuConcurrencyTest)  Time elapsed: 0.404 sec  <<< FAILURE!
java.lang.AssertionError: 
Expecting:
 <java.util.concurrent.CancellationException>
to be an instance of:
 <java.io.IOException>
but was instance of:
 <java.util.concurrent.CancellationException>
	at org.corfudb.AbstractCorfuConcurrencyTest.concurrentTestsThrowExceptions(AbstractCorfuConcurrencyTest.java:37)

@no2chem
Copy link
Member Author

no2chem commented Dec 8, 2016

@slfritchie Okay, and that was because the test was expecting an IOException, but the test was cancelled before the exception was thrown...

@no2chem no2chem self-assigned this Dec 8, 2016
@no2chem no2chem added this to the v1.0 Release milestone Dec 8, 2016
Copy link
Member

@dahliamalkhi dahliamalkhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeh, useful constants. I suspect we'll refine this as we go along, but at this point, any step towards defining and unifying timeouts/iterations as constants is a good step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants