Skip to content

Enable defining custom test environments#391

Merged
eric-maynard merged 3 commits intoapache:mainfrom
andrew4699:aguterman-dynamic-test-environment
Nov 4, 2024
Merged

Enable defining custom test environments#391
eric-maynard merged 3 commits intoapache:mainfrom
andrew4699:aguterman-dynamic-test-environment

Conversation

@andrew4699
Copy link
Contributor

@andrew4699 andrew4699 commented Oct 22, 2024

Description

This is a test-only change which adds a pattern for running tests against user-specified environments by specifying your own base URL and HTTP client. Makes PolarisRestCatalogViewIntegrationTest use the custom test environment. In a future PR I plan to do similar for other tests but I'd like this PR to just start the pattern.

Example:

INTEGRATION_TEST_BASE_URL="http://localhost:8181" \
INTEGRATION_TEST_HTTP_CLIENT_FACTORY_IMPL="org.apache.polaris.MyCustomHttpClientFactory" \
INTEGRATION_TEST_ROLE_ARN="arn:aws:iam::<role>" \
INTEGRATION_TEST_S3_PATH="s3://<bucket>" \
./gradlew test --info --rerun --tests "org.apache.polaris.service.catalog.PolarisRestCatalogViewIntegrationTest"

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • Existing tests work
  • Tested running against a non-Dropwizard Polaris with a custom HTTP client

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

* clients. This should be a class name. If this is not set, falls back to the client created by
* Dropwizard.
*/
public static final String ENV_HTTP_CLIENT_FACTORY_IMPL =
Copy link
Contributor

Choose a reason for hiding this comment

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

This approach seems a bit roundabout. Is there a way we can achieve this with a service file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For things like this the project seems to have relied on providing a test config file and Jackson parsing the config. I'm hoping to decouple this from Dropwizard and for non-Dropwizard environments also not force people to go through that whole dance.

Copy link
Contributor

@eric-maynard eric-maynard left a comment

Choose a reason for hiding this comment

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

Left a few questions

@andrew4699 andrew4699 force-pushed the aguterman-dynamic-test-environment branch 2 times, most recently from 3bac154 to 0afb1a3 Compare October 29, 2024 22:27
@andrew4699 andrew4699 force-pushed the aguterman-dynamic-test-environment branch 2 times, most recently from 458320e to c81e4a3 Compare October 30, 2024 17:38
Copy link
Contributor

@eric-maynard eric-maynard 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!

@andrew4699 andrew4699 force-pushed the aguterman-dynamic-test-environment branch from 76bc3f5 to 1229ab9 Compare November 4, 2024 17:34
@eric-maynard eric-maynard enabled auto-merge (squash) November 4, 2024 17:39
@eric-maynard eric-maynard merged commit b292c40 into apache:main Nov 4, 2024
@andrew4699 andrew4699 deleted the aguterman-dynamic-test-environment branch November 4, 2024 17:53
@dimas-b dimas-b mentioned this pull request Feb 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants