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

[CI] CronEvalToolTests testEnsureDateIsShownInRootLocale sometimes fails #35687

Closed
droberts195 opened this issue Nov 19, 2018 · 6 comments · Fixed by #38394
Closed

[CI] CronEvalToolTests testEnsureDateIsShownInRootLocale sometimes fails #35687

droberts195 opened this issue Nov 19, 2018 · 6 comments · Fixed by #38394
Assignees
Labels
:Data Management/Watcher >test-failure Triaged test failures from CI

Comments

@droberts195
Copy link
Contributor

CronEvalToolTests testEnsureDateIsShownInRootLocale sometimes fails. An example is https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+matrix-java-periodic/ES_BUILD_JAVA=java11,ES_RUNTIME_JAVA=java11,nodes=virtual&&linux/67/consoleText

The exception is:

   > Throwable #1: java.lang.AssertionError: 
   > Expected: a string containing "] in UTC, local time is"
   >      but: was "Valid!
   > Now is [Mon, 19 Nov 2018 05:20:04] in UTC
   > Here are the next 1 times this cron expression will trigger:
   > 1.	Mon, 2 Jan 2040 11:00:00
   > "

The , local time is bit is missing.

This did not reproduce locally for me:

./gradlew :x-pack:plugin:watcher:test \
  -Dtests.seed=204FA128FA77A81C \
  -Dtests.class=org.elasticsearch.xpack.watcher.trigger.schedule.tool.CronEvalToolTests \
  -Dtests.method="testEnsureDateIsShownInRootLocale" \
  -Dtests.security.manager=true \
  -Dtests.locale=ar-SS \
  -Dtests.timezone=Asia/Seoul \
  -Dcompiler.java=11 \
  -Druntime.java=11
@droberts195 droberts195 added >test-failure Triaged test failures from CI :Data Management/Watcher labels Nov 19, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@droberts195
Copy link
Contributor Author

Test is muted in master in 2ee25c8

@astefan astefan self-assigned this Feb 1, 2019
@astefan
Copy link
Contributor

astefan commented Feb 1, 2019

I was able to reproduce this via Eclipse only (it seems gradle won't let me pass the same in the command line) by passing -Duser.timezone=UTC to the VM starting the test. The way the test does the UTC timezone comparison is a bit different than what the Watcher code does it:

  • the test goes straight to java.util.TimeZone.getDefault()
  • Watcher code uses org.joda.time.DateTimeZone.forTimeZone(null) which first looks at user.timezone system property and, if that one doesn't exist, it takes the timezone from TimeZone.getDefault().

Not sure how would this scenario reach this stage so that the comparison fails... somehow user.timezone must have been set on the box the test ran in which made the test to fail...

@spinscale @rjernst my investigation above does sound right to you? Is it worth changing the test to use the same approach for local timezone - UTC comparison as CronEvalTool?

@astefan
Copy link
Contributor

astefan commented Feb 1, 2019

Reproduceable with gradle command: gradlew :x-pack:plugin:watcher:test -Dtests.seed=204FA128FA77A81C -Dtests.class=org.elasticsearch.xpack.watcher.trigger.schedule.tool.CronEvalToolTests -Dtests.method="testEnsureDateIsShownInRootLocale" -Dtests.security.manager=true -Dtests.locale=ar-SS -Dtests.timezone=UTC -Dcompiler.java=11 -Druntime.java=11 -Dtests.jvm.argline="-Duser.timezone=UTC"

@pgomulka
Copy link
Contributor

pgomulka commented Feb 4, 2019

@astefan @spinscale when using java-time ZoneId.systemDefault in both test and CronEvalTool the behaviour will be as suggested by Andrei

to use the same approach for local timezone - UTC comparison as CronEvalTool?
I will enable this test once my main PR #35809 is merged

pgomulka added a commit to pgomulka/elasticsearch that referenced this issue Feb 5, 2019
The test is now expected to be always passing no matter what the random
locale is. This is fixed with using jdk ZoneId.systemDefault() in both
the test and CronEvalTool

closes elastic#35687
pgomulka added a commit that referenced this issue Feb 5, 2019
The test is now expected to be always passing no matter what the random
locale is. This is fixed with using jdk ZoneId.systemDefault() in both
the test and CronEvalTool

closes #35687
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Watcher >test-failure Triaged test failures from CI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants