Skip to content

Commit

Permalink
unittests: increase timeouts and fix race in DiskSpaceAllocatorTest
Browse files Browse the repository at this point in the history
Various unit tests have timeouts to allow the test to fail if it gets
stuck.  While such tests are short-lived and the existing timeouts work
fine for desktop environments, when testing under heavy load the thread
scheduling within the OS can lead to severe delays and the tests could
fail.  For this reason, the timeouts are bumped up to 1 minute.  Tests
should aim to complete easily within one second (number picked out of
the air), so one minute should allow at least 2 orders of magnitude
leeway.

This patch also removes two unnecessary assertions.  These assertions
add nothing against an existing, earlier test and introduce a race
condition for a possible failure mode.

The test in question has a race in attempting to ensure the testing
thread blocks.  This is hard to fix without rewriting the code being
tested and, since it doesn't affect the test result, the code is left
as-is.

Target: master
Patch: https://rb.dcache.org/r/7308/
Acked-by: Tigran Mkrtchyan
Requires-notes: no
Requires-book: no
Request: 2.10
Request: 2.9
Request: 2.8
Request: 2.7
Request: 2.6
  • Loading branch information
paulmillar committed Sep 24, 2014
1 parent 8d749c0 commit 235237b
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ public void shouldFailForGetParentOfMissingEntry() throws Exception



@Test(timeout=10000)
@Test(timeout=60_000)
public void shouldSucceedForSuccessfulListWithSingleReplyMessage() throws Exception
{
givenListResponses(
Expand All @@ -482,7 +482,7 @@ public void shouldSucceedForSuccessfulListWithSingleReplyMessage() throws Except
assertThat(file2Attr.getPnfsId(), is(ANOTHER_PNFSID));
}

@Test(timeout=10000)
@Test(timeout=60_000)
public void shouldSucceedForSuccessfulListWithTwoReplyMessage() throws Exception
{
givenListResponses(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ public void testUpdateState() throws InterruptedException {
_maintainer.countPendingUpdates());
}

@Test(timeout = 1000)
@Test(timeout = 60_000)
public void testCountPendingUpdates() throws InterruptedException {
// Create a caretaker that will take 10s to process each request.
SlowCaretaker slowCaretaker = new SlowCaretaker( 1000);
Expand All @@ -256,7 +256,7 @@ public void testCountPendingUpdates() throws InterruptedException {
_maintainer.countPendingUpdates());
}

@Test(timeout = 1000)
@Test(timeout = 60_000)
public void testUpdateProcessed() throws InterruptedException {
_caretaker.setProcessUpdateCount( 1);

Expand All @@ -275,7 +275,7 @@ public void testUpdateProcessed() throws InterruptedException {
_maintainer.countPendingUpdates());
}

@Test(timeout = 1000)
@Test(timeout = 60_000)
public void testTwoUpdateProcessed() throws InterruptedException {
_caretaker.setProcessUpdateCount( 2);

Expand Down Expand Up @@ -303,7 +303,7 @@ public void testTwoUpdateProcessed() throws InterruptedException {
*
*/

@Test(timeout = 1000)
@Test(timeout = 60_000)
public void testExpiring() throws InterruptedException {

/**
Expand All @@ -322,7 +322,7 @@ public void testExpiring() throws InterruptedException {

// See http://rt.dcache.org/Ticket/Display.html?id=7330
@Ignore("Broken test: depends on timing on machine")
@Test(timeout = 1000)
@Test(timeout = 60_000)
public void testExpiringDelayChanging() throws InterruptedException {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public void testAllocateMoreThanFreeNonBlock() throws Exception {
}


@Test(timeout=500)
@Test(timeout=60_000)
public void testAllocateWithFree() throws Exception {

final Random random = new Random();
Expand All @@ -171,12 +171,8 @@ public void testAllocateWithFree() throws Exception {
assertEquals("used space do not match allocated space", allocSize, account.getUsed());
assertEquals("free space do not match after alloc",space - allocSize, account.getFree());

final long timeout = 200; // 0.2 seconds
DiskSpaceAllocationTestHelper.delayedFreeEntry(account, allocSize, timeout);

// Should be true.
assertEquals("used space do not match allocated space", allocSize, account.getUsed());
assertEquals("free space do not match after alloc",space - allocSize, account.getFree());
long delay = 200; // 0.2 seconds
DiskSpaceAllocationTestHelper.delayedFreeEntry(account, allocSize, delay);

// Should block until space is freed.
account.allocate(space);
Expand Down Expand Up @@ -255,7 +251,7 @@ public void testSetTotalDecMissing() throws Exception {
}


@Test(timeout=400)
@Test(timeout=60_000)
public void testAllocateWithSetTotalInc() throws Exception {

final long initialTotalSize = 1000;
Expand Down

0 comments on commit 235237b

Please sign in to comment.