LanguageUtilsTest -- increase the tolerance in the assertions #623

Merged
merged 1 commit into from Mar 22, 2013

Conversation

Projects
None yet
3 participants
@buildhive

This comment has been minimized.

Show comment
Hide comment
@buildhive

buildhive Mar 22, 2013

Brooklyn Central » brooklyn #318 FAILURE
Looks like there's a problem with this pull request
(what's this?)

Brooklyn Central » brooklyn #318 FAILURE
Looks like there's a problem with this pull request
(what's this?)

@buildhive

This comment has been minimized.

Show comment
Hide comment
@buildhive

buildhive Mar 22, 2013

Brooklyn Central » brooklyn #319 FAILURE
Looks like there's a problem with this pull request
(what's this?)

Brooklyn Central » brooklyn #319 FAILURE
Looks like there's a problem with this pull request
(what's this?)

@aledsage

View changes

core/src/test/java/brooklyn/util/internal/LanguageUtilsTest.groovy
- assertTrue System.currentTimeMillis() > start+50
- assertTrue System.currentTimeMillis() < start+500
+ assertTrue System.currentTimeMillis() > start+10
+ assertTrue System.currentTimeMillis() < start+200

This comment has been minimized.

@aledsage

aledsage Mar 22, 2013

Member

This has made the tolerance even less: < start+200 is stricter than < start+500. I'd write something like:

    long start = System.currentTimeMillis();
    LanguageUtils.repeatUntilSuccess(period: 1*TimeUnit.MILLISECONDS, timeout: 5000*TimeUnit.MILLISECONDS) { 
        System.currentTimeMillis() > start+50 
    }
    long time = System.currentTimeMillis() - start;
    assertTrue(time >= 50, "time="+time);
    assertTrue(time <= 1000, "time="+time);
@aledsage

aledsage Mar 22, 2013

Member

This has made the tolerance even less: < start+200 is stricter than < start+500. I'd write something like:

    long start = System.currentTimeMillis();
    LanguageUtils.repeatUntilSuccess(period: 1*TimeUnit.MILLISECONDS, timeout: 5000*TimeUnit.MILLISECONDS) { 
        System.currentTimeMillis() > start+50 
    }
    long time = System.currentTimeMillis() - start;
    assertTrue(time >= 50, "time="+time);
    assertTrue(time <= 1000, "time="+time);
@aledsage

This comment has been minimized.

Show comment
Hide comment
@aledsage

aledsage Mar 22, 2013

Member

Will see if buildhive thinks this is good enough to fix the test failure, and then merge.

Member

aledsage commented Mar 22, 2013

Will see if buildhive thinks this is good enough to fix the test failure, and then merge.

@buildhive

This comment has been minimized.

Show comment
Hide comment
@buildhive

buildhive Mar 22, 2013

Brooklyn Central » brooklyn #320 FAILURE
Looks like there's a problem with this pull request
(what's this?)

Brooklyn Central » brooklyn #320 FAILURE
Looks like there's a problem with this pull request
(what's this?)

@aledsage

This comment has been minimized.

Show comment
Hide comment
@aledsage

aledsage Mar 22, 2013

Member

The test passed in buildhive, but then failed at:

CliTest.testLaunchCommandParsesArgs:188 LaunchCommand{verbose=false, quiet=false, app=my.App, script=null, location=localhost, port=8081+, noConsole=false, noShutdownOnExit=false, stopOnKeyPress=false} expected [true] but found [false]

So merging.

Member

aledsage commented Mar 22, 2013

The test passed in buildhive, but then failed at:

CliTest.testLaunchCommandParsesArgs:188 LaunchCommand{verbose=false, quiet=false, app=my.App, script=null, location=localhost, port=8081+, noConsole=false, noShutdownOnExit=false, stopOnKeyPress=false} expected [true] but found [false]

So merging.

aledsage added a commit that referenced this pull request Mar 22, 2013

Merge pull request #623 from andreaturli/fix/language-utils-test
LanguageUtilsTest -- increase the tolerance in the assertions

@aledsage aledsage merged commit 45cfe9a into brooklyncentral:master Mar 22, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment