Fix HistoryIntegrationTests timestamp comparsion

[pgomulka]

When the millisecond part of a timestamp is 0 the toString
representation in java-time is omitting the millisecond part (joda was
not). The Search response is returning timestamps formatted with
WatcherDateTimeUtils, therefore comparisons of strings should be done
with the same formatter

relates #27330

[elasticmachine]

Pinging @elastic/es-core-infra

[pgomulka]

This was spotted in a PR Build https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+pull-request-2/7451/console

REPRODUCE WITH: ./gradlew :x-pack:plugin:watcher:unitTest -Dtests.seed=7C3F86FEA0E888ED -Dtests.class=org.elasticsearch.xpack.watcher.test.integration.HistoryIntegrationTests -Dtests.method="testThatHistoryContainsStatus" -Dtests.security.manager=true -Dtests.locale=cs -Dtests.timezone=Asia/Chungking -Dcompiler.java=11 -Druntime.java=8

However the ClockMock has to be locally modified to have constructor fixing the time with milliseconds being 0:

 public ClockMock() {
        this(Clock.systemUTC());
setTime(DateFormatters.from(DateFormatter.forPattern("date_time").parse("2019-02-05T23:32:52.000Z")));
    }

[cbuescher]

The changes look good to me. However I'm wondering if we could change the test to be reproducable just using the random seed. Its not ideal that this test currently only fails when overwriting the ClockMock constructor. Maybe there is a way to initialize the clock used in tests with a date that depends on the random seed only and then freeze it? I don't know if this is possible in this test but I think it would be a great improvement. Could be done in a follow up PR though.

[cbuescher]

@pgomulka just to follow up on my last comment, the ClockMock seems to be initalized by the TimeWarpedWatcher plugin used in watcher tests.
I have a feeling the TimeWarp which contains the clock can be accessed in AbstractWatcherIntegrationTestCase#timeWarp() and from there it
should be possible to get the clock and fix it to a date depending on the random seed. Maybe this should be done just in this test method, maybe its something
that might be useful in a @before block fixing time for everything in HistoryIntegrationTests, but I don't know. Maybe a frozen clock doesn't work with some
of the other tests in this suite.

[pgomulka]

@cbuescher yes ideally it would be great to make the time depending on the seed we provide, for instance

  public ClockMock() {
        this(Clock.systemUTC());
        int millis = LuceneTestCase.random().nextInt(1000);// what bound
        setTime(Instant.ofEpochMilli(millis));
    }

but then we would need to be sure that all the usages of ZonedDateTime.now are using a this clock, otherwise the clock would be in 1970's and other parts of code in 2019

[pgomulka]

I think this would end up a much bigger change, better to tackle that in a separate PR. We are having similar discussion on this PR: #38473

[cbuescher]

I think this would end up a much bigger change, better to tackle that in a separate PR.

Agreed, I'm also not sure if it would be feasible in the case of HistoryIntegrationTests. I'll open an issue about it to track it though.

[cbuescher]

@pgomulka can you add a version label and some sort of scope (I think its the ">test-failure" or sth. similar) to this so this gets sorted right for the release changes?

[cbuescher]

I opened #38520 to track the question of missing reproducability here.