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

Extend the IT framework to allow tests in extensions #13877

Merged
merged 53 commits into from
May 15, 2023

Conversation

paul-rogers
Copy link
Contributor

@paul-rogers paul-rogers commented Mar 2, 2023

The "new" IT framework provides a convenient way to package and run integration tests (ITs), but only for core modules. We have a use case to run an IT for a contrib extension: the proposed gRPC query extension. This PR provides the IT framework functionality to allow non-core ITs.

The it.sh script takes an option third parameter to specify the relative path to the Maven module that holds the test:

./it.sh test MyCategory extensions-contrib/my-extension-it

Because of the Maven build order, the ITs for an extension must be in a Maven module separate from the extension itself. In this case, my-extension is the extension, while my-extension-it are the ITs for that extension.

The layout in the extension mimics the existing cases module. Provide a Maven file that runs the tests using FailSafe. Provide a cluster directory structured as explained in the IT docs.

Extra Extensions

Contrib extensions are not added to the base Docker image. They can still be used in a test:

  • Create an extensions directory in the shared folder (mounted at /shared in the container.)
  • Unpack the extension to this directory so you have extensions/my-extn.
  • Add the absolute path to the extension in your docker_compose.py file: /shared/extensions/my-extn

Revision to IT Structure

As part of this work, several changes were made to the structure of the existing IT directory.

  • The Python "template" for Docker Compose moves from templates/Category.py to cluster/Category/docker-compose.py.
  • A new 'cluster/Category/verify.shscript peforms pre-test checks. The awkward code that was in.it.shto check for required environment variables moved intoverify.sh`.
  • A new cluster/Category/setup.sh script does any additional setup needed before launching an IT cluster. Use this to add additional files to the shared directory, for example. Using the script is far easier than trying to script in Maven.
  • The cluster startup and shutdown steps were removed from cases/pom.xml. We were not using them: we now use it.sh to do this work.
  • The it.sh and cluster.sh scripts are now aware of two modules: the "base" cases module, and the "test" module. Before now, those were the same. Not they can differ.

Prevent Snapshot Downloading

Also added configuration to the root pom.xml file to prevent an attempt to download snapshots from upstream. This most often happens when dependencies are missing and Maven cannot find a required Druid module, and instead tries to ask upstream repos. Bad Things would happen if it actually found such jars.

Release note

Added the druid.extensions.path config setting. This setting is optional and is normally not set. Primarily for testing. See configuration/index.md for details.


This PR has:

  • been self-reviewed.
  • added internal documentation for new or modified features or behaviors.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added integration tests.
  • been tested in an integration test Druid cluster.

paul-rogers and others added 28 commits January 14, 2023 14:36
Report stats & close statement
Simple server + query test works
The gRPC shaded jar module causes issues when loaded
into an IDE. This PR hides the module from Maven (and
hence IDEs) by building it implicitly in the grpc-query
module. It is a hack, but it works.
add empty response and GrpcResponseHandler tests
Allow ITs outside of the 'cases' directory
Refactoring
Build the grpc-query extension archive
Build a jar for the test Protobuf message
Fixes to the extension path mechanism
* Refactor template into cluster directory
* Add verify, setup scripts
* Forbid snapshots from upstream repos
* Basic grpc-query IT test
* run command to only run tests
* test always stops containers
* prune -> prune-volumnes
* prune-containers to stop all running containers
* github prunes containers and volumes
Copy link
Contributor

@abhishekrb19 abhishekrb19 left a comment

Choose a reason for hiding this comment

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

Thanks for the detailed code comments. LGTM.

docs/configuration/index.md Outdated Show resolved Hide resolved
@paul-rogers
Copy link
Contributor Author

This PR is blocked on a failure in the Kafka tests. Debugging failed to determine if a change in this PR caused the failure, or if the tests are just reliably flaky. Kafka failures seem uncorrelated to the changes made while debugging, and the tests fail so often it is hard to determine when a code path might be clean. Let's just put this on hold until the Kafka issue is resolved.

@paul-rogers paul-rogers marked this pull request as draft March 24, 2023 18:21
@paul-rogers paul-rogers mentioned this pull request Apr 4, 2023
8 tasks
@paul-rogers paul-rogers marked this pull request as ready for review April 4, 2023 19:33
@paul-rogers
Copy link
Contributor Author

The Kafka tests have failed for the last month. At first I thought it was because the PR contained a change to the way Druid locates extensions. But, I removed that code (having found another solution to adding test-specific extensions), yet Kafka still fails. I suspect the test itself is flaky. The test was converted to the new IT framework to allow debugging (in a separate PR). However, the test ran perfectly on the local machine after the conversion. So, let's just assume the Kafka test is flaky and ignore it.

@paul-rogers
Copy link
Contributor Author

The gRPC query PR, #14024, depends on this code. In fact, this PR was pulled out of (an early version of) the gRPC extension. Once this PR is merged, we will merge master into the rRPC PR to remove these files from the rRPC PR.

# Conflicts:
#	integration-tests-ex/cases/pom.xml
Copy link
Member

@tejaswini-imply tejaswini-imply left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, @paul-rogers. LGTM.

@abhishekagarwal87 abhishekagarwal87 merged commit 3c0983c into apache:master May 15, 2023
@abhishekagarwal87 abhishekagarwal87 added this to the 27.0 milestone Jul 19, 2023
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.

6 participants