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

Improve reproducability of HistoryIntegrationTests #38520

Closed
cbuescher opened this issue Feb 6, 2019 · 6 comments
Closed

Improve reproducability of HistoryIntegrationTests #38520

cbuescher opened this issue Feb 6, 2019 · 6 comments
Labels
:Data Management/Watcher Team:Data Management Meta label for data/management team >test Issues or PRs that are addressing/adding tests

Comments

@cbuescher
Copy link
Member

While fixing and issue in HistoryIntegrationTests#testThatHistoryContainsStatus in #38505 it became clear that the initial issue wasn't reproducible using just the random test seed because it uses some current System time via the ClockMock. Ideally we should be able to reproduce issues by supplying just the test seed when re-running the test.
I'd like open this issue to investigate ideas and the feasability to improve this, maybe only for HistoryIntegrationTests but also maybe more generally for other Watcher or time-dependent tests.

Maybe this isn't possible for HistoryIntegrationTests because the nature of the things tested require some sort of advancing clock, but maybe parts of it can be controlled by a reproducible random source. See #38505 (comment) for some more context.

@cbuescher cbuescher added >test Issues or PRs that are addressing/adding tests :Data Management/Watcher labels Feb 6, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@pgomulka
Copy link
Contributor

pgomulka commented Feb 8, 2019

there is much more which that are covering code using ZonedDateTime.now(..) or System.currentTimeMillis or System.nanoTime
One solution would be to refactor all these calls to use Clock interface which would have to be injected and could be mocked for testing purposes.
The refactoring effort would be huge though

@pgomulka
Copy link
Contributor

pgomulka commented Feb 8, 2019

Also worth noting that for watcher tests this can be solved easier as there is already a mechanism to mocking clock from AbstractWatcherIntegrationTestCase.timeWarp
We could try at least refactoring this for watcher..

@pgomulka
Copy link
Contributor

This was also discussed on core-infra meeting on 13th Feb. The conclusion is to focus on one specific use case where the clock can be mocked.
Not only time can be nondeterministic but concurrency as well. Some code might be executed different number of times depending on concurrency, so it is very hard to have a generic mechanism

@rjernst rjernst added the Team:Data Management Meta label for data/management team label May 4, 2020
@cbuescher
Copy link
Member Author

@pgomulka I'm just going through some open issues of mine. Do you think its useful to keep this one open? The general goal of mocking clock to make tests reproducable seems clear, but there seems no clear definition on when this goal is reached, so IMHO having this task sit here doesn't add much value for us. I wouldn't mind closing if you agree.

@pgomulka
Copy link
Contributor

totally agree - let's close it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Watcher Team:Data Management Meta label for data/management team >test Issues or PRs that are addressing/adding tests
Projects
None yet
Development

No branches or pull requests

4 participants