Skip to content

Correct assertions on profiling action cancel#92046

Merged
danielmitterdorfer merged 3 commits intoelastic:mainfrom
danielmitterdorfer:fix-92039
Dec 2, 2022
Merged

Correct assertions on profiling action cancel#92046
danielmitterdorfer merged 3 commits intoelastic:mainfrom
danielmitterdorfer:fix-92039

Conversation

@danielmitterdorfer
Copy link
Copy Markdown
Member

With this commit we address two issues when testing cancellation of the profiling task:

  1. We add the actual profiling task to the list of tasks to check, not only its child tasks.
  2. The original test contained a race where child tasks might have been unregistered (expectedly) in between collecting tasks and checking for cancelled tasks. Now we only check all remaining tasks have been cancelled.

Closes #92039

With this commit we address two issues when testing cancellation of the
profiling task:

1. We add the actual profiling task to the list of tasks to check, not
   only its child tasks.
2. The original test contained a race where child tasks might have been
   unregistered (expectedly) in between collecting tasks and checking
for cancelled tasks. Now we only check all remaining tasks have been
cancelled.

Closes elastic#92039
@danielmitterdorfer danielmitterdorfer added >test Issues or PRs that are addressing/adding tests :Search/Search Search-related issues that do not fall into other categories v8.7.0 labels Dec 1, 2022
@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Dec 1, 2022
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-search (Team:Search)

@DaveCTurner
Copy link
Copy Markdown
Member

I've muted this test for now in main since it seems to fail enough to warrant it and Luca might not get to this review today. I've also pushed a commit to this PR branch which unmutes it.

Copy link
Copy Markdown
Contributor

@javanna javanna left a comment

Choose a reason for hiding this comment

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

LGTM thanks a lot for jumping on this.

@danielmitterdorfer
Copy link
Copy Markdown
Member Author

Thanks for the review! There was one test failure in elasticsearch-ci/packaging-tests-windows-sample:


18:45:39 Could not determine the dependencies of task ':x-pack:plugin:ml:explodedBundlePlugin'.
18:45:39 > Could not resolve all files for configuration ':x-pack:plugin:ml:nativeBundle'.
18:45:39    > Could not download ml-cpp-8.7.0-SNAPSHOT-deps.zip (org.elasticsearch.ml:ml-cpp:8.7.0-SNAPSHOT)
18:45:39       > Could not get resource 'https://artifacts-snapshot.elastic.co/ml-cpp/8.7.0-SNAPSHOT/downloads/ml-cpp/ml-cpp-8.7.0-SNAPSHOT-deps.zip'.
18:45:39          > Premature end of Content-Length delimited message body (expected: 340,418,397; received: 2,097,136)

As this change is completely unrelated, the failure above is transient and all subsequent builds of elasticsearch-ci/packaging-tests-windows-sample, I'll merge this now.

@danielmitterdorfer danielmitterdorfer merged commit a721c66 into elastic:main Dec 2, 2022
@danielmitterdorfer danielmitterdorfer deleted the fix-92039 branch December 2, 2022 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team >test Issues or PRs that are addressing/adding tests v8.7.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] GetProfilingActionIT.testAutomaticCancellation

4 participants