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

EvilLoggerTests.testDeprecatedSettings fails assertion #25680

Closed
talevy opened this issue Jul 12, 2017 · 11 comments
Closed

EvilLoggerTests.testDeprecatedSettings fails assertion #25680

talevy opened this issue Jul 12, 2017 · 11 comments
Assignees
@jasontedor
Labels
>test Issues or PRs that are addressing/adding tests v5.6.0

Comments

@talevy
Copy link
Contributor

talevy commented Jul 12, 2017 •
edited
Loading

looks like a consistent behavior where-in this test fails on Java 9 in CI

console logs:
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+5.x+java9-periodic/3193/console
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+5.x+java9-periodic/3196/console
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+5.x+java9-periodic/3198/console

FAILURE 0.16s J0 | EvilLoggerTests.testDeprecatedSettings <<< FAILURES!
   > Throwable #1: java.lang.AssertionError: 
   > Expected: <1>
   >      but: was <0>
   > 	at __randomizedtesting.SeedInfo.seed([FA22CEC1F9E81B67:9A5B351BD2070DE8]:0)
   > 	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
   > 	at org.elasticsearch.common.logging.EvilLoggerTests.testDeprecatedSettings(EvilLoggerTests.java:271)
   > 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   > 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   > 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   > 	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
   > 	at java.base/java.lang.Thread.run(Thread.java:844)
  2> NOTE: leaving temporary files on disk at: /var/lib/jenkins/workspace/elastic+elasticsearch+5.x+java9-periodic/qa/evil-tests/build/testrun/test/J0/temp/org.elasticsearch.common.logging.EvilLoggerTests_FA22CEC1F9E81B67-001
  2> NOTE: test params are: codec=Asserting(Lucene62): {}, docValues:{}, maxPointsInLeafNode=699, maxMBSortInHeap=6.526490608214228, sim=RandomSimilarity(queryNorm=false,coord=no): {}, locale=dz, timezone=Asia/Kamchatka
  2> NOTE: Linux 4.8.0-53-generic amd64/Oracle Corporation 9 (64-bit)/cpus=8,threads=1,free=333207680,total=536870912
  2> NOTE: All tests run in this JVM: [PluginSecurityTests, ESPolicyUnitTests, EvilSecurityTests, EvilLoggerTests]
Completed [14/14] on J0 in 4.81s, 10 tests, 1 failure <<< FAILURES!

reproduce with (I cannot reproduce):

gradle :qa:evil-tests:test -Dtests.seed=6F4D930464F6F74A -Dtests.class=org.elasticsearch.common.logging.EvilLoggerTests -Dtests.method="testDeprecatedSettings" -Dtests.security.manager=false -Dtests.locale=en-IE -Dtests.timezone=Universal
@talevy talevy added :Delivery/Build Build or test infrastructure jdk9 v5.6.0 >test Issues or PRs that are addressing/adding tests and removed :Delivery/Build Build or test infrastructure labels Jul 12, 2017
@talevy
Copy link
Contributor Author

talevy commented Aug 9, 2017

happened again... not in java9

https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+5.6+oracle-periodic/157/console

@talevy talevy changed the title EvilLoggerTests.testDeprecatedSettings fails in Java 9 EvilLoggerTests.testDeprecatedSettings fails assertion Aug 9, 2017
@talevy talevy removed the jdk9 label Aug 9, 2017
@andyb-elastic
Copy link
Contributor

Another one in 5.6 https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+5.6+aggressive-opts/49/console

I'll awaitsfix this test since it's happened a handful of times this week

andyb-elastic added a commit that referenced this issue Aug 11, 2017
@andyb-elastic
AwaitFix testDeprecatedSettings (#25680)
17e41d7
@jasontedor
Copy link
Member

This one is so incredibly puzzling. I can get it to reproduce spuriously, but only on 5.6. Still no idea what is going on. 😐

@jasontedor
Copy link
Member

I think I found the issue, it's from interaction with another test.

@jasontedor
Copy link
Member

And it only happens on 5.6 because the interaction is from deprecated use of path.scripts, which does not exist in 6.0, 6.x, and master.

@jasontedor
Copy link
Member

I have this one nailed down. It uncovered an issue with #20575 and #24076, I will open a pull request for this first. Then I will address this test failure.

@talevy
Copy link
Contributor Author

talevy commented Aug 14, 2017

sweet! so I'll drop the 6.1.0 tag

@talevy talevy removed the v6.1.0 label Aug 14, 2017
@jasontedor
Copy link
Member

Step 1: #26209

@jasontedor
Copy link
Member

Step 2: #26210

@jasontedor
Copy link
Member

Step 3: b1f6131

jasontedor added a commit that referenced this issue Aug 15, 2017
@jasontedor
Lazily resolve path.scripts setting
b1f6131 
This commit introduces lazy resolving of the path.scripts setting. This
is necessary because the current implementation resolves path.scripts in
the constructor for the environment. The problem here is that:
 - path.scripts is deprecated
 - the act of getting this setting in the constructor for the
   environment triggers deprecation logging
 - constructing the environment happens before logging is configured
 - performing deprecation logging before logging is configured fails the
   node on startup

Therefore, lazy resolving path.scripts so that construction of the
environment can proceed, then configuring logging, then finally
resolving this setting enables getting past this issue.

Additionally, we fix an evil security test that was causing an evil
logging test to fail. The problem here is that if the evil security test
executed before and in the same JVM as the evil logging test, then since
the test was using path.scripts it would trigger deprecation logging
before logging had been configured which would lead to deprecation
logging being disabled. Since the evil logging test was relying on
deprecation logging being enabled, this would lead to test failures. To
fix this, we simply do not set path.scripts in the evil security
test. This is not a major change in test behavior since the test was
merely explicitly setting the value of path.scripts to the default value
anyway.

Closes #25680
@jasontedor
Copy link
Member

Closed by b1f6131

@jasontedor jasontedor mentioned this issue Aug 15, 2017
Closed
@colings86 colings86 added v5.6.0 and removed v5.6.1 labels Aug 24, 2017
@jkakavas jkakavas mentioned this issue Nov 28, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jasontedor jasontedor

Labels
>test Issues or PRs that are addressing/adding tests v5.6.0
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@clintongormley @colings86 @talevy @jasontedor @andyb-elastic