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

Add integration test framework #687

Merged

Conversation

ryanbogan
Copy link
Member

Description

Adds an integration test framework, utilizing the changes made in opensearch-project/OpenSearch#7235. The integration test task starts the helloWorld extension, spins up a test single-node cluster to initialize the extensions, and then kills the cluster and the extension process running in the background. There are currently no tests written for this framework, so the task will fail with an error temporarily.

Note: Must be merged after opensearch-project/OpenSearch#7235

Issues Resolved

#589

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@dblock
Copy link
Member

dblock commented Apr 25, 2023

Is this the first time a consumer of opensearch-sdk-java would be bringing in the gradle tasks from OpenSearch core? That feels like undesirable. The alternative would be to (re)implement a simpler version of the functionality of test cluster setup in the SDK.

@owaiskazi19 owaiskazi19 mentioned this pull request Apr 28, 2023
@saratvemulapalli
Copy link
Member

Is this the first time a consumer of opensearch-sdk-java would be bringing in the gradle tasks from OpenSearch core? That feels like undesirable. The alternative would be to (re)implement a simpler version of the functionality of test cluster setup in the SDK.

We definitely could implement one in SDK. We've taken this path to:

  • Not duplicate code for test cluster setup
  • Plugins migrating to extensions have all the features from the framework
  • Seamless integration test support with opensearch-build

I definitely understand we'd like to keep the SDK as independent as possible, we'd definitely take it as a follow up.

@saratvemulapalli
Copy link
Member

@ryanbogan whats needed here to get this moving?

@ryanbogan
Copy link
Member Author

@saratvemulapalli Working on fixing merge conflicts now. I also want to test it out with the new protobuf additions but I'll have everything finalized by EOD

@ryanbogan
Copy link
Member Author

I'm not sure why gradle check is failing for a dependency conflict. We have the same version as OpenSearch and everything is passing on my local.

build.gradle Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
@ryanbogan ryanbogan requested a review from joshpalis May 25, 2023 16:13
Signed-off-by: Ryan Bogan <[email protected]>
Copy link
Member

@joshpalis joshpalis 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 for the changes Ryan.

@ryanbogan ryanbogan requested a review from owaiskazi19 May 30, 2023 16:02
Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

LGTM! A few config changes outside the scope of this that I'll save for another PR/issue.

@owaiskazi19
Copy link
Member

The path for the integTest folder should be under a directory called test -> src/test/java/org/opensearch/sdk/test/integTest/TransportCommunicationIT.java.

@owaiskazi19 owaiskazi19 merged commit 04d9eab into opensearch-project:main May 31, 2023
@ryanbogan ryanbogan deleted the add_integ_test_framework branch May 31, 2023 21:31
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 31, 2023
* Add integration test task that uses the OpenSearch gradle plugin

Signed-off-by: Ryan Bogan <[email protected]>

* Added closeTestExtension task to kill testExtension process

Signed-off-by: Ryan Bogan <[email protected]>

* Fix spotless

Signed-off-by: Ryan Bogan <[email protected]>

* Implement resolution strategy to fix dependency conflict

Signed-off-by: Ryan Bogan <[email protected]>

* Minor adjustment

Signed-off-by: Ryan Bogan <[email protected]>

* Change jackson version

Signed-off-by: Ryan Bogan <[email protected]>

* Change jackson dataformat version

Signed-off-by: Ryan Bogan <[email protected]>

* Revert jackson version change

Signed-off-by: Ryan Bogan <[email protected]>

* Add resolution strategy to fix jackson dependency conflict

Signed-off-by: Ryan Bogan <[email protected]>

* Address PR Comments

Signed-off-by: Ryan Bogan <[email protected]>

* Add new implementation to fix error from separate merge

Signed-off-by: Ryan Bogan <[email protected]>

* Address PR Comments

Signed-off-by: Ryan Bogan <[email protected]>

* Address PR Comments

Signed-off-by: Ryan Bogan <[email protected]>

* Revert minor change

Signed-off-by: Ryan Bogan <[email protected]>

* Address PR Comments

Signed-off-by: Ryan Bogan <[email protected]>

* Add reading from yaml for testExtensionCreation

Signed-off-by: Ryan Bogan <[email protected]>

* Spotless

Signed-off-by: Ryan Bogan <[email protected]>

* Minor change

Signed-off-by: Ryan Bogan <[email protected]>

* Debug test commit

Signed-off-by: Ryan Bogan <[email protected]>

* Minor changes

Signed-off-by: Ryan Bogan <[email protected]>

* Remove extra test class

Signed-off-by: Ryan Bogan <[email protected]>

* Add index

Signed-off-by: Ryan Bogan <[email protected]>

* Fix inadvertent change

Signed-off-by: Ryan Bogan <[email protected]>

* Minor change

Signed-off-by: Ryan Bogan <[email protected]>

* Add log4j resolution strategy

Signed-off-by: Ryan Bogan <[email protected]>

* Remove testing println

Signed-off-by: Ryan Bogan <[email protected]>

---------

Signed-off-by: Ryan Bogan <[email protected]>
Co-authored-by: Sarat Vemulapalli <[email protected]>
Co-authored-by: Owais Kazi <[email protected]>
(cherry picked from commit 04d9eab)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
owaiskazi19 added a commit that referenced this pull request Jun 1, 2023
owaiskazi19 added a commit that referenced this pull request Jun 1, 2023
This reverts commit 04d9eab.

Signed-off-by: owaiskazi19 <[email protected]>
dbwiddis pushed a commit that referenced this pull request Jun 7, 2023
* Add integration test framework (#687)

* Add integration test task that uses the OpenSearch gradle plugin

Signed-off-by: Ryan Bogan <[email protected]>

* Added closeTestExtension task to kill testExtension process

Signed-off-by: Ryan Bogan <[email protected]>

* Fix spotless

Signed-off-by: Ryan Bogan <[email protected]>

* Implement resolution strategy to fix dependency conflict

Signed-off-by: Ryan Bogan <[email protected]>

* Minor adjustment

Signed-off-by: Ryan Bogan <[email protected]>

* Change jackson version

Signed-off-by: Ryan Bogan <[email protected]>

* Change jackson dataformat version

Signed-off-by: Ryan Bogan <[email protected]>

* Revert jackson version change

Signed-off-by: Ryan Bogan <[email protected]>

* Add resolution strategy to fix jackson dependency conflict

Signed-off-by: Ryan Bogan <[email protected]>

* Address PR Comments

Signed-off-by: Ryan Bogan <[email protected]>

* Add new implementation to fix error from separate merge

Signed-off-by: Ryan Bogan <[email protected]>

* Address PR Comments

Signed-off-by: Ryan Bogan <[email protected]>

* Address PR Comments

Signed-off-by: Ryan Bogan <[email protected]>

* Revert minor change

Signed-off-by: Ryan Bogan <[email protected]>

* Address PR Comments

Signed-off-by: Ryan Bogan <[email protected]>

* Add reading from yaml for testExtensionCreation

Signed-off-by: Ryan Bogan <[email protected]>

* Spotless

Signed-off-by: Ryan Bogan <[email protected]>

* Minor change

Signed-off-by: Ryan Bogan <[email protected]>

* Debug test commit

Signed-off-by: Ryan Bogan <[email protected]>

* Minor changes

Signed-off-by: Ryan Bogan <[email protected]>

* Remove extra test class

Signed-off-by: Ryan Bogan <[email protected]>

* Add index

Signed-off-by: Ryan Bogan <[email protected]>

* Fix inadvertent change

Signed-off-by: Ryan Bogan <[email protected]>

* Minor change

Signed-off-by: Ryan Bogan <[email protected]>

* Add log4j resolution strategy

Signed-off-by: Ryan Bogan <[email protected]>

* Remove testing println

Signed-off-by: Ryan Bogan <[email protected]>

---------

Signed-off-by: Ryan Bogan <[email protected]>
Co-authored-by: Sarat Vemulapalli <[email protected]>
Co-authored-by: Owais Kazi <[email protected]>
(cherry picked from commit 04d9eab)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Change versions and resolve conflicts from original PR

Signed-off-by: Ryan Bogan <[email protected]>

* Fix dependency conflicts

Signed-off-by: Ryan Bogan <[email protected]>

* Fix snapshot version

Signed-off-by: Ryan Bogan <[email protected]>

* Combined commit for backporting 'Fix Publishing' PR's

Signed-off-by: Ryan Bogan <[email protected]>

---------

Signed-off-by: Ryan Bogan <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Sarat Vemulapalli <[email protected]>
Co-authored-by: Owais Kazi <[email protected]>
Co-authored-by: Ryan Bogan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants