Skip to content

[ILM] TEST: fix long overflow in TimeValueScheduleTests #36384

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 10, 2018
Merged

[ILM] TEST: fix long overflow in TimeValueScheduleTests #36384

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions ...ugin/ilm/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeValueScheduleTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,15 @@
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.EqualsHashCodeTestUtils;
import org.junit.Before;

import java.util.concurrent.TimeUnit;

public class TimeValueScheduleTests extends ESTestCase {

private long start;
private TimeValue interval;

public TimeValueSchedule createRandomInstance() {
return new TimeValueSchedule(createRandomTimeValue());
}
Expand All @@ -22,7 +26,15 @@ private TimeValue createRandomTimeValue() {
return new TimeValue(randomLongBetween(1, 10000), randomFrom(TimeUnit.SECONDS, TimeUnit.MINUTES, TimeUnit.HOURS, TimeUnit.DAYS));
}

public void testHascodeAndEquals() {
@Before
public void setUpStartAndInterval() {
// start with random epoch between 1/1/1970 and 31/12/2035 so that start is not
// so large such that (start + interval) > Long.MAX
start = randomLongBetween(0, 2082672000000L);
Copy link
Contributor

@gwbrown gwbrown Dec 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe I'm missing something but I don't quite get the math here - this number + 10,000 days (from createRandomTimeValue) is way less than Long.MAX (2^63 - 1)? Are you just giving it plenty of headroom?

Copy link
Contributor Author

@talevy talevy Dec 7, 2018

Choose a reason for hiding this comment

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

yeah, why not be more restrictive :). doing randomNonnegativeLong was too large of a range

Copy link
Contributor Author

@talevy talevy Dec 7, 2018

Choose a reason for hiding this comment

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

thought I would leave a comment to explain why the magic numbers

Copy link
Contributor

@gwbrown gwbrown Dec 7, 2018

Choose a reason for hiding this comment

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

Alright, cool - I just read this the first time as saying it had to be 31/12/2035 (or less) to prevent an overflow, rather than that being somewhat arbitrary.

interval = createRandomTimeValue();
}

public void testHashcodeAndEquals() {
for (int i = 0; i < 20; i++) {
EqualsHashCodeTestUtils.checkEqualsAndHashCode(createRandomInstance(),
instance -> new TimeValueSchedule(instance.getInterval()),
Expand All @@ -31,34 +43,26 @@ public void testHascodeAndEquals() {
}

public void testNextScheduledTimeFirstTriggerNotReached() {
long start = randomNonNegativeLong();
TimeValue interval = createRandomTimeValue();
long triggerTime = start + interval.millis();
long now = start + randomLongBetween(0, interval.millis() - 1);
TimeValueSchedule schedule = new TimeValueSchedule(interval);
assertEquals(triggerTime, schedule.nextScheduledTimeAfter(start, now));
}

public void testNextScheduledTimeAtFirstInterval() {
long start = randomNonNegativeLong();
TimeValue interval = createRandomTimeValue();
long triggerTime = start + 2 * interval.millis();
long now = start + interval.millis();
TimeValueSchedule schedule = new TimeValueSchedule(interval);
assertEquals(triggerTime, schedule.nextScheduledTimeAfter(start, now));
}

public void testNextScheduledTimeAtStartTime() {
long start = randomNonNegativeLong();
TimeValue interval = createRandomTimeValue();
long triggerTime = start + interval.millis();
TimeValueSchedule schedule = new TimeValueSchedule(interval);
assertEquals(triggerTime, schedule.nextScheduledTimeAfter(start, start));
}

public void testNextScheduledTimeAfterFirstTrigger() {
long start = randomNonNegativeLong();
TimeValue interval = createRandomTimeValue();
long numberIntervalsPassed = randomLongBetween(0, 10000);
long triggerTime = start + (numberIntervalsPassed + 1) * interval.millis();
long now = start
Expand Down