honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Harden painless test against "fun" caching (#24077) 

The JVM caches `Integer` objects. This is known. A test in Painless
was relying on the JVM not caching the particular integer `1000`.
It turns out that when you provide `-XX:+AggressiveOpts` the JVM
*does* cache `1000`, causing the test to fail when that is
specified.

This replaces `1000` with a randomly selected integer that we test
to make sure *isn't* cached by the JVM. *Hopefully* this test is
good enough. It relies on the caching not changing in between when
we check that the value isn't cached and when we run the painless
code. The cache now is a simple array but there is nothing
preventing it from changing. If it does change in a way that thwarts
this test then the test fail fail again. At least when that happens
the next person can see the comment about how it is important
that the integer isn't cached and can follow that line of inquiry.

Closes #24041
This commit is contained in:
Nik Everett 2017-04-17 13:44:05 -04:00 committed by GitHub
parent 34eda1a1a8
commit 25119a7e78
1 changed files with 19 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
modules/lang-painless/src/test/java/org/elasticsearch/painless/EqualsTests.java
View File

@ -1,5 +1,3 @@
package org.elasticsearch.painless;
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
@ -19,7 +17,11 @@ package org.elasticsearch.painless;
* under the License.
*/
import org.apache.lucene.util.Constants;
package org.elasticsearch.painless;
import org.elasticsearch.test.ESTestCase;
import static java.util.Collections.singletonMap;
// TODO: Figure out a way to test autobox caching properly from methods such as Integer.valueOf(int);
public class EqualsTests extends ScriptTestCase {
@ -132,11 +134,13 @@ public class EqualsTests extends ScriptTestCase {
}
public void testBranchEqualsDefAndPrimitive() {
assumeFalse("test fails on Windows", Constants.WINDOWS);
assertEquals(true, exec("def x = 1000; int y = 1000; return x == y;"));
assertEquals(false, exec("def x = 1000; int y = 1000; return x === y;"));
assertEquals(true, exec("def x = 1000; int y = 1000; return y == x;"));
assertEquals(false, exec("def x = 1000; int y = 1000; return y === x;"));
/* This test needs an Integer that isn't cached by Integer.valueOf so we draw one randomly. We can't use any fixed integer because
* we can never be sure that the JVM hasn't configured itself to cache that Integer. It is sneaky like that. */
int uncachedAutoboxedInt = randomValueOtherThanMany(i -> Integer.valueOf(i) == Integer.valueOf(i), ESTestCase::randomInt);
assertEquals(true, exec("def x = params.i; int y = params.i; return x == y;", singletonMap("i", uncachedAutoboxedInt), true));
assertEquals(false, exec("def x = params.i; int y = params.i; return x === y;", singletonMap("i", uncachedAutoboxedInt), true));
assertEquals(true, exec("def x = params.i; int y = params.i; return y == x;", singletonMap("i", uncachedAutoboxedInt), true));
assertEquals(false, exec("def x = params.i; int y = params.i; return y === x;", singletonMap("i", uncachedAutoboxedInt), true));
}
public void testBranchNotEquals() {
@ -150,11 +154,13 @@ public class EqualsTests extends ScriptTestCase {
}
public void testBranchNotEqualsDefAndPrimitive() {
assumeFalse("test fails on Windows", Constants.WINDOWS);
assertEquals(false, exec("def x = 1000; int y = 1000; return x != y;"));
assertEquals(true, exec("def x = 1000; int y = 1000; return x !== y;"));
assertEquals(false, exec("def x = 1000; int y = 1000; return y != x;"));
assertEquals(true, exec("def x = 1000; int y = 1000; return y !== x;"));
/* This test needs an Integer that isn't cached by Integer.valueOf so we draw one randomly. We can't use any fixed integer because
* we can never be sure that the JVM hasn't configured itself to cache that Integer. It is sneaky like that. */
int uncachedAutoboxedInt = randomValueOtherThanMany(i -> Integer.valueOf(i) == Integer.valueOf(i), ESTestCase::randomInt);
assertEquals(false, exec("def x = params.i; int y = params.i; return x != y;", singletonMap("i", uncachedAutoboxedInt), true));
assertEquals(true, exec("def x = params.i; int y = params.i; return x !== y;", singletonMap("i", uncachedAutoboxedInt), true));
assertEquals(false, exec("def x = params.i; int y = params.i; return y != x;", singletonMap("i", uncachedAutoboxedInt), true));
assertEquals(true, exec("def x = params.i; int y = params.i; return y !== x;", singletonMap("i", uncachedAutoboxedInt), true));
}
public void testRightHandNull() {