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

Remove a couple of awaitsfix in EsqlActionIT #100391

Closed
wants to merge 1 commit into from

Conversation

ChrisHegarty
Copy link
Contributor

This commit removes a couple of awaits fixes in EsqlActionIT that are no longer needed - previous changes have resolved the underlying issue and the test scenarios now run successfully consistently.

@ChrisHegarty ChrisHegarty added >test Issues or PRs that are addressing/adding tests Team:QL (Deprecated) Meta label for query languages team :Analytics/ES|QL AKA ESQL labels Oct 6, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-ql (Team:QL)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/elasticsearch-esql (:Query Languages/ES|QL)

@ChrisHegarty
Copy link
Contributor Author

Note: currently the complete EsqlActionIT test is marked awaits fix, but the test will be reenabled soon by #100370.

@ChrisHegarty
Copy link
Contributor Author

Argh! :-(

loopy testing has now hit this:

REPRODUCE WITH: ./gradlew 'null' --tests "org.elasticsearch.xpack.esql.action.EsqlActionIT.testFromLimit [seed=[362F331FBB1FF941:1155810CE0590A89]]" -Dtests.seed=362F331FBB1FF941 -Dtests.locale=sk-SK -Dtests.timezone=Europe/Mariehamn -Druntime.java=19

java.lang.AssertionError: Request breaker not reset to 0 on node: node_s0
Expected: <0L>
     but: was <64L>
Expected :<0L>
Actual   :<64L>
at __randomizedtesting.SeedInfo.seed([362F331FBB1FF941:1155810CE0590A89]:0)
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
at org.junit.Assert.assertThat(Assert.java:956)
at org.elasticsearch.test.InternalTestCluster.lambda$ensureEstimatedStats$41(InternalTestCluster.java:2439)
at org.elasticsearch.test.ESTestCase.assertBusy(ESTestCase.java:1196)
at org.elasticsearch.test.ESTestCase.assertBusy(ESTestCase.java:1169)
at org.elasticsearch.test.InternalTestCluster.ensureEstimatedStats(InternalTestCluster.java:2437)
at org.elasticsearch.test.TestCluster.assertAfterTest(TestCluster.java:92)
at org.elasticsearch.test.InternalTestCluster.assertAfterTest(InternalTestCluster.java:2486)
at org.elasticsearch.test.ESIntegTestCase.afterInternal(ESIntegTestCase.java:590)
....

Copy link
Contributor

@bpintea bpintea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@dnhatn
Copy link
Member

dnhatn commented Oct 6, 2023

REPRODUCE WITH: ./gradlew 'null' --tests "org.elasticsearch.xpack.esql.action.EsqlActionIT.testFromLimit [seed=[362F331FBB1FF941:1155810CE0590A89]]" -Dtests.seed=362F331FBB1FF941 -Dtests.locale=sk-SK -Dtests.timezone=Europe/Mariehamn -Druntime.java=19

I have a theory and I will look into this.

@ChrisHegarty
Copy link
Contributor Author

closing in favour of splitting these two Awaits fixes, since one of them is stable, the other is not yet. See #100604

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL Team:QL (Deprecated) Meta label for query languages team >test Issues or PRs that are addressing/adding tests v8.11.0 v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants