Bind testing server to 0.0.0.0 #172
Conversation
Also, use bespoke S3 client for testing
| delegatingFacade.setDelegate(new PathStyleRemoteS3Facade((_, _) -> httpErrorResponseServer.getBaseUrl().getHost(), false, Optional.of(httpErrorResponseServer.getBaseUrl().getPort()))); | ||
| } | ||
|
|
||
| @AfterEach |
There was a problem hiding this comment.
Is the constructor invoked once per test? It's not obvious from the test whether it's @TestInstance(PER_CLASS) or @TestInstance(PER_METHOD). I'd suggest annotating the class to make it explicit.
There was a problem hiding this comment.
@TrinoAwsProxyTest implies per class (and other settings).
@Retention(RUNTIME)
@Target(TYPE)
@Inherited
@ExtendWith(TrinoAwsProxyTestExtension.class)
@TestInstance(PER_CLASS)
There was a problem hiding this comment.
Hmm... if its @TestInstance(PER_CLASS), then it's not going to work with this shutdown method, which is going to be called after each individual test. You need to use @AfterAll, instead.
| internalClient = clientBuilder(httpErrorResponseServer.getBaseUrl()) | ||
| .forcePathStyle(true) | ||
| .credentialsProvider(() -> AwsSessionCredentials.create(testingCredentials.emulated().accessKey(), testingCredentials.emulated().secretKey(), testingCredentials.emulated().session().orElse(""))) | ||
| .build(); | ||
| delegatingFacade.setDelegate(new PathStyleRemoteS3Facade((_, _) -> httpErrorResponseServer.getBaseUrl().getHost(), false, Optional.of(httpErrorResponseServer.getBaseUrl().getPort()))); |
There was a problem hiding this comment.
In this state, this doesn't test anything, right?
The point was to test that aws-proxy propagates errors from the remote S3 correctly.
What this is doing currently is bypassing the aws-proxy and calling httpErrorResponseServer with an S3 client
There was a problem hiding this comment.
Derp - I messed it up I think. Feel free to submit a fixup PR. I probably didn't understand the test.
Changes added in trinodb#172 meant to fix an issue with the IP `createTestingHttpServer` binds on accidentally broke this test
Also, use bespoke S3 client for testing