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.

Sign up for GitHub

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

Jump to bottom

Correct InterleavedExternalTargetRequestsIT teardown #514

Merged
merged 3 commits into from
Jun 16, 2021
Merged

Correct InterleavedExternalTargetRequestsIT teardown #514

merged 3 commits into from
Jun 16, 2021

Conversation

andrewazores
Copy link
Member

Fixes #501

@andrewazores
Set failsafe runOrder to random
61eb4fa
@andrewazores
Query Cryostat for targets to ensure complete setup/teardown
9655a99 
JDP async discovery takes time to find targets and time to notice targets have gone away. Test setup/teardown methods repeatedly query Cryostat until the number of discovered targets reaches the expected state. This ensures that the test methods run with all the targets present, and ensures that after the tests are completed the instance under test is reset to a fresh state before continuing on to the next integration test class.
@andrewazores andrewazores requested a review from ebaron June 15, 2021 20:10
ebaron
ebaron approved these changes Jun 15, 2021
View reviewed changes
Copy link
Member

@ebaron ebaron left a comment

Choose a reason for hiding this comment

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

Looks good! Hopefully it's an easy backport to v1.

@andrewazores
Copy link
Member Author

Looks good! Hopefully it's an easy backport to v1.

I'll take a look at that. v1 still has the original "ConnectToExternalTargetsIT" without interleaved requests, since the concurrent TargetConnectionManager patch was also never backported to v1. I can try to backport that concurrent connections patch, which includes the interleaving update to the integration test, but I'm a little hesitant about that since it's a fairly significant change to the TargetConnectionManager, and that's a pretty key piece of internal infrastructure. Otherwise I can just manually apply these setup/teardown changes to the older version of the ConnectToExternalTargetsIT, which should be easy enough.

@andrewazores andrewazores mentioned this pull request Jun 16, 2021
Merged
@andrewazores andrewazores merged commit f9017b3 into cryostatio:main Jun 16, 2021
@andrewazores andrewazores deleted the issue-501-external-targets-itest-teardown branch June 16, 2021 02:08
andrewazores added a commit to andrewazores/cryostat that referenced this pull request Jun 24, 2021
@andrewazores
Correct InterleavedExternalTargetRequestsIT teardown (cryostatio#514)
78491f3 
* Set failsafe runOrder to random

* Query Cryostat for targets to ensure complete setup/teardown

JDP async discovery takes time to find targets and time to notice targets have gone away. Test setup/teardown methods repeatedly query Cryostat until the number of discovered targets reaches the expected state. This ensures that the test methods run with all the targets present, and ensures that after the tests are completed the instance under test is reset to a fresh state before continuing on to the next integration test class.

* Apply spotless formatting
aali309 pushed a commit to aali309/cryostat-legacy that referenced this pull request Jul 22, 2024
@mwangggg
chore(logs): audit log levels (cryostatio#514)
cf35ce3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebaron ebaron ebaron approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ConnectToExternalTargetsIT does not properly clean up
2 participants
@andrewazores @ebaron