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

Fix byte size value equals/hash code test #29643

Merged
merged 2 commits into from
Apr 24, 2018

Conversation

jasontedor
Copy link
Member

This commit fixes two issues with the byte size value equals/hash code test.

The first problem is due to a test failure when the original instance is zero bytes and we pick the mutation branch where we preserve the size but change the unit. The mutation should result in a different byte size value but changing the unit on zero bytes still leaves us with zero bytes.

During the course of fixing this test I discovered another problem. When we need to randomize size, we could randomly select a size that would lead to an overflow of Long.MAX_VALUE.

This commit fixes both of these issues.

This commit fixes two issues with the byte size value equals/hash code
test.

The first problem is due to a test failure when the original instance is
zero bytes and we pick the mutation branch where we preserve the size
but change the unit. The mutation should result in a different byte size
value but changing the unit on zero bytes still leaves us with zero
bytes.

During the course of fixing this test I discovered another problem. When
we need to randomize size, we could randomly select a size that would
lead to an overflow of Long.MAX_VALUE.

This commit fixes both of these issues.
if (instanceSize == 0 || instanceSize >= Long.MAX_VALUE / newUnitBytes) {
mutateSize = randomValueOtherThanMany(
v -> v == instanceSize && v >= Long.MAX_VALUE / newUnitBytes, () -> randomNonNegativeLong() / newUnitBytes);
} else {
Copy link
Member Author

Choose a reason for hiding this comment

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

The crux of the change is this if block. I added a condition that we need to randomize size of the input size is zero (otherwise we would end up with zero bytes equals zero bytes but the mutation is suppose to have a different value). Additionally, I added a condition to not randomly select a value whose bytes representation would exceed Long.MAX_VALUE.

@jasontedor
Copy link
Member Author

run the sample packaging tests

@jasontedor
Copy link
Member Author

I should mention, this is the reproduction that fails prior to this fix and passes now:

./gradlew :server:test \
  -Dtests.seed=F6F30441CCF5E770 \
  -Dtests.class=org.elasticsearch.common.unit.ByteSizeValueTests \
  -Dtests.method="testEqualsAndHashcode" \
  -Dtests.security.manager=true \
  -Dtests.locale=no \
  -Dtests.timezone=America/Creston

Copy link
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

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

LGTM

@colings86 colings86 added >test Issues or PRs that are addressing/adding tests :Core/Infra/Core Core issues without another label labels Apr 24, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jasontedor jasontedor merged commit 25e45a7 into elastic:master Apr 24, 2018
jasontedor added a commit that referenced this pull request Apr 24, 2018
This commit fixes two issues with the byte size value equals/hash code
test.

The first problem is due to a test failure when the original instance is
zero bytes and we pick the mutation branch where we preserve the size
but change the unit. The mutation should result in a different byte size
value but changing the unit on zero bytes still leaves us with zero
bytes.

During the course of fixing this test I discovered another problem. When
we need to randomize size, we could randomly select a size that would
lead to an overflow of Long.MAX_VALUE.

This commit fixes both of these issues.
jasontedor added a commit that referenced this pull request Apr 24, 2018
This commit fixes two issues with the byte size value equals/hash code
test.

The first problem is due to a test failure when the original instance is
zero bytes and we pick the mutation branch where we preserve the size
but change the unit. The mutation should result in a different byte size
value but changing the unit on zero bytes still leaves us with zero
bytes.

During the course of fixing this test I discovered another problem. When
we need to randomize size, we could randomly select a size that would
lead to an overflow of Long.MAX_VALUE.

This commit fixes both of these issues.
jasontedor added a commit that referenced this pull request Apr 24, 2018
This commit fixes two issues with the byte size value equals/hash code
test.

The first problem is due to a test failure when the original instance is
zero bytes and we pick the mutation branch where we preserve the size
but change the unit. The mutation should result in a different byte size
value but changing the unit on zero bytes still leaves us with zero
bytes.

During the course of fixing this test I discovered another problem. When
we need to randomize size, we could randomly select a size that would
lead to an overflow of Long.MAX_VALUE.

This commit fixes both of these issues.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 25, 2018
* master:
  [Test] Fix docs check for DEB package in packaging tests (elastic#30126)
  Painless: Docs Clean Up (elastic#29592)
  Fixes Eclipse build for sql jdbc project (elastic#30114)
  Remove reference to `not_analyzed`.
  [Docs] Add community analysis plugin (elastic#29612)
  Remove the suggest metric from stats APIs (elastic#29635)
  Fix byte size value equals/hash code test (elastic#29643)
  Upgrade to Gradle 4.7 (elastic#29644)
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 25, 2018
* master:
  [Test] Fix docs check for DEB package in packaging tests (elastic#30126)
  Painless: Docs Clean Up (elastic#29592)
  Fixes Eclipse build for sql jdbc project (elastic#30114)
  Remove reference to `not_analyzed`.
  [Docs] Add community analysis plugin (elastic#29612)
  Remove the suggest metric from stats APIs (elastic#29635)
  Fix byte size value equals/hash code test (elastic#29643)
  Upgrade to Gradle 4.7 (elastic#29644)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >test Issues or PRs that are addressing/adding tests v6.2.5 v6.3.0 v6.4.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants