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.

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

Jump to bottom

Painless EqualsTest#testBranch(Not)?EqualsDefAndPrimitive fails on Windows #24041

Closed
jasontedor opened this issue Apr 11, 2017 · 6 comments
Closed

Painless EqualsTest#testBranch(Not)?EqualsDefAndPrimitive fails on Windows #24041

jasontedor opened this issue Apr 11, 2017 · 6 comments
Assignees
@nik9000
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >test Issues or PRs that are addressing/adding tests

Comments

@jasontedor
Copy link
Member

jasontedor commented Apr 11, 2017 •
edited
Loading

These two tests are currently failing on Windows only.
@jasontedor jasontedor added :Plugin Lang Painless >test Issues or PRs that are addressing/adding tests labels Apr 11, 2017
@jasontedor
Copy link
Member Author

To silence these tests on Windows I pushed:
5.3: d0a7961
5.x: 0800005
master: 6536190

@jasontedor jasontedor changed the title Painless EqualsTests#testBranchEqualsDefAndPrimitive and EqualsTests#testBranchNotEqualsDefAndPrimitive Painless EqualsTest#testBranch(Not)?EqualsDefAndPrimitive fails on Windows Apr 11, 2017
@nik9000
Copy link
Member

nik9000 commented Apr 11, 2017 via email

@nik9000
Copy link
Member

nik9000 commented Apr 12, 2017

This didn't reproduce on windows 10 so I'm trying out "Windows 2012 r2" which is the image that this failure occurred on:
http://build-us-00.elastic.co/job/ES%20Server%205.x%20on%20Windows%202012%20R2/15/consoleFull#gradle-task-1686

@nik9000
Copy link
Member

nik9000 commented Apr 12, 2017

Didn't reproduce for me with the same windows version and the same jvm version. I'm not really sure what is up. I'm going to go away for a few days but when I come back I might reenable and have another look.

The failures look to be on gradle 3.5 so maybe it is #24072 ? Not sure.

@jasontedor
Copy link
Member Author

We have failures on 3.3 also.

=======================================
Elasticsearch Build Hamster says Hello!
=======================================
  Gradle Version        : 3.3
  OS Info               : Windows Server 2012 R2 6.3 (amd64)
  JDK Version           : Oracle Corporation 1.8.0_60 [Java HotSpot(TM) 64-Bit Server VM 25.60-b23]
  JAVA_HOME             : C:\Program Files\Java\jdk1.8.0_60
Incremental java compilation is an incubating feature.
.
.
.
[ant:junit4] JVM J3: stdout was not empty, see: C:\jenkins\workspace\ES Server 5.x on Windows 2012 R2\modules\lang-painless\build\testrun\test\temp\junit4-J3-20170409_202447_5686557531052253476795.sysout
Tests with failures:
  - org.elasticsearch.painless.EqualsTests.testBranchEqualsDefAndPrimitive
  - org.elasticsearch.painless.EqualsTests.testBranchNotEqualsDefAndPrimitive

Slow Tests Summary:
 12.69s | org.elasticsearch.painless.NeedsScoreTests
  7.50s | org.elasticsearch.painless.RemainderTests
  5.94s | org.elasticsearch.painless.ReservedWordTests
  4.99s | org.elasticsearch.painless.ScriptEngineTests
  4.94s | org.elasticsearch.painless.ScoreTests

[ant:junit4] JVM J0:     0.82 ..    15.46 =    14.63s
[ant:junit4] JVM J1:     0.82 ..    15.63 =    14.81s
[ant:junit4] JVM J2:     0.82 ..    15.89 =    15.06s
[ant:junit4] JVM J3:     0.82 ..    23.78 =    22.95s
[ant:junit4] Execution time total: 23 seconds
[ant:junit4] Tests summary: 46 suites, 749 tests, 2 failures, 1 ignored (1 assumption)
:modules:lang-painless:test FAILED
:modules:lang-painless:test (Thread[main,5,main]) completed. Took 23.858 secs.

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':modules:lang-painless:test'.
> There were test failures: 46 suites, 749 tests, 2 failures, 1 ignored (1 assumption) [seed: 4C7AA2AB6C0DF051]

* Try:
Run with --stacktrace option to get the stack trace. Run with --debug option to get more log output.

BUILD FAILED

Total time: 49 mins 32.547 secs
Stopped 1 worker daemon(s).

@nik9000
Copy link
Member

nik9000 commented Apr 12, 2017

Discovered. If you provide -XX:+AggressiveOpts then the test will fail. It relies on the size of the Integer cache. I can now reliably fail locally.

And now I'm going to go away for a few days. So I can write the fix but can't merge it because I won't be about to baby sit it.

nik9000 added a commit to nik9000/elasticsearch that referenced this issue Apr 12, 2017
@nik9000
Harden painless test against "fun" caching
268e0b6 
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 elastic#24041
@nik9000 nik9000 mentioned this issue Apr 12, 2017
Merged
nik9000 added a commit that referenced this issue Apr 17, 2017
@nik9000
Harden painless test against "fun" caching (#24077)
25119a7 
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
nik9000 added a commit that referenced this issue Apr 17, 2017
@nik9000
Harden painless test against "fun" caching (#24077)
313547b 
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
nik9000 added a commit that referenced this issue Apr 21, 2017
@nik9000
Harden painless test against "fun" caching (#24077)
43e0445 
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
@clintongormley clintongormley added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache and removed :Plugin Lang Painless labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nik9000 nik9000

Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >test Issues or PRs that are addressing/adding tests
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@clintongormley @nik9000 @jasontedor