Skip to content

Conversation

@albertzaharovits
Copy link
Contributor

@albertzaharovits albertzaharovits commented Feb 14, 2025

In test-scoped internal ITs the cluster().assertAfterTest() method was invoked after the cluster nodes were closed. Consequently, the assertions that iterated over the internal nodes (and asserted some state on nodes after the test) were all noops (again, just in the case of test-scoped internal ITs)!
This PR reverses that order, so that after test assertions are effective again.

NB: This PR technically changes the order for external clusters too, but those are deprecated now and not used in a test-scoped way, from what I can tell.

@albertzaharovits albertzaharovits self-assigned this Feb 14, 2025
@albertzaharovits albertzaharovits changed the title Assert after test before cluster close Invoke TestCluster#assertAfterTest before closing the cluster Feb 14, 2025
@albertzaharovits albertzaharovits added :Core/Infra/Core Core issues without another label >test Issues or PRs that are addressing/adding tests v9.0.1 labels Feb 14, 2025
@albertzaharovits albertzaharovits marked this pull request as ready for review February 14, 2025 19:29
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Feb 14, 2025
Copy link
Contributor

@prdoyle prdoyle left a comment

Choose a reason for hiding this comment

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

Hard to believe this was deliberate.

@prdoyle prdoyle added the auto-backport Automatically create backport pull requests when merged label Feb 14, 2025
@albertzaharovits albertzaharovits added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) and removed auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels Feb 15, 2025
@elasticsearchmachine elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Feb 15, 2025
@albertzaharovits
Copy link
Contributor Author

@elasticsearchmachine run Elasticsearch Serverless Checks

@albertzaharovits albertzaharovits enabled auto-merge (squash) February 16, 2025 06:24
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.18
8.x
9.0

albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Feb 16, 2025
…c#122639)

In test-scoped internal ITs the `cluster().assertAfterTest()` method was invoked
*after* the cluster nodes were closed. Consequently, the assertions that iterated
over the internal nodes (and asserted some state on nodes after the test) were
all effectively noops.
This PR reverses that order, so that after-test assertions are effective again.
albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Feb 16, 2025
…c#122639)

In test-scoped internal ITs the `cluster().assertAfterTest()` method was invoked
*after* the cluster nodes were closed. Consequently, the assertions that iterated
over the internal nodes (and asserted some state on nodes after the test) were
all effectively noops.
This PR reverses that order, so that after-test assertions are effective again.
albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Feb 16, 2025
…c#122639)

In test-scoped internal ITs the `cluster().assertAfterTest()` method was invoked
*after* the cluster nodes were closed. Consequently, the assertions that iterated
over the internal nodes (and asserted some state on nodes after the test) were
all effectively noops.
This PR reverses that order, so that after-test assertions are effective again.
elasticsearchmachine pushed a commit that referenced this pull request Feb 16, 2025
… (#122710)

In test-scoped internal ITs the `cluster().assertAfterTest()` method was invoked
*after* the cluster nodes were closed. Consequently, the assertions that iterated
over the internal nodes (and asserted some state on nodes after the test) were
all effectively noops.
This PR reverses that order, so that after-test assertions are effective again.
elasticsearchmachine pushed a commit that referenced this pull request Feb 16, 2025
… (#122709)

In test-scoped internal ITs the `cluster().assertAfterTest()` method was invoked
*after* the cluster nodes were closed. Consequently, the assertions that iterated
over the internal nodes (and asserted some state on nodes after the test) were
all effectively noops.
This PR reverses that order, so that after-test assertions are effective again.
elasticsearchmachine pushed a commit that referenced this pull request Feb 16, 2025
… (#122711)

In test-scoped internal ITs the `cluster().assertAfterTest()` method was invoked
*after* the cluster nodes were closed. Consequently, the assertions that iterated
over the internal nodes (and asserted some state on nodes after the test) were
all effectively noops.
This PR reverses that order, so that after-test assertions are effective again.
@albertzaharovits albertzaharovits deleted the es-integ-test-case-after-test branch February 16, 2025 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Core/Infra/Core Core issues without another label serverless-linked Added by automation, don't add manually Team:Core/Infra Meta label for core/infra team >test Issues or PRs that are addressing/adding tests v8.18.1 v8.19.0 v9.0.1 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants