You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "damondouglas (via GitHub)" <gi...@apache.org> on 2023/11/02 00:27:07 UTC
[PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
damondouglas opened a new pull request, #29262:
URL: https://github.com/apache/beam/pull/29262
This PR closes #28710 by implementing a test implementation of Caller and SetupTeardown for the [mock-api](https://github.com/apache/beam/tree/master/.test-infra/mock-apis) endpoint.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://github.com/apache/beam/blob/master/CONTRIBUTING.md#make-the-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger workflows.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "damondouglas (via GitHub)" <gi...@apache.org>.
damondouglas commented on code in PR #29262:
URL: https://github.com/apache/beam/pull/29262#discussion_r1383797998
##########
sdks/java/io/rrio/src/test/java/org/apache/beam/io/requestresponse/EchoHTTPCallerTestIT.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.io.requestresponse;
+
+import static org.apache.beam.sdk.io.common.IOITHelper.readIOTestPipelineOptions;
+import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import com.google.protobuf.ByteString;
+import java.net.URI;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoRequest;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoResponse;
+import org.apache.beam.testinfra.mockapis.echo.v1.EchoServiceGrpc;
+import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests for {@link EchoHTTPCallerTestIT} on a deployed {@link EchoServiceGrpc} instance's HTTP
+ * handler. See {@link EchoITOptions} for details on the required parameters and how to provide
+ * these for running integration tests.
+ */
+@RunWith(JUnit4.class)
+public class EchoHTTPCallerTestIT {
+
+ private static @MonotonicNonNull EchoITOptions options;
+ private static @MonotonicNonNull EchoHTTPCaller client;
+ private static final ByteString PAYLOAD = ByteString.copyFromUtf8("payload");
+
+ @BeforeClass
+ public static void setUp() throws UserCodeExecutionException {
+ options = readIOTestPipelineOptions(EchoITOptions.class);
+ if (options.getHttpEndpointAddress().isEmpty()) {
+ throw new RuntimeException(
+ "--httpEndpointAddress is missing. See " + EchoITOptions.class + "for details.");
+ }
+ client = EchoHTTPCaller.of(URI.create(options.getHttpEndpointAddress()));
+
+ EchoRequest request = createShouldExceedQuotaRequest();
+
+ // Quota allocation needs at least 1 so we invoke a few times to guarantee exceeding quota.
+ // We only throw exceptions that are not UserCodeQuotaException.
+ try {
+ EchoResponse ignored = client.call(request);
+ client.call(request);
+ client.call(request);
+ client.call(request);
+ } catch (UserCodeExecutionException ignored) {
+ }
+ }
+
+ @Test
+ public void givenValidRequest_receivesResponse() throws UserCodeExecutionException {
+ EchoRequest request = createShouldNeverExceedQuotaRequest();
+ EchoResponse response = client.call(request);
+ assertEquals(response.getId(), request.getId());
+ assertEquals(response.getPayload(), request.getPayload());
+ }
+
+ @Test
+ public void givenExceededQuota_shouldThrow() {
+ assertThrows(UserCodeQuotaException.class, () -> client.call(createShouldExceedQuotaRequest()));
+ }
+
+ @Test
+ public void givenNotFound_shouldThrow() {
+ UserCodeExecutionException error =
+ assertThrows(
+ UserCodeExecutionException.class,
+ () ->
+ client.call(
+ EchoRequest.newBuilder()
+ .setId("i-dont-exist-quota-id")
+ .setPayload(PAYLOAD)
+ .build()));
+
+ assertTrue(error.getMessage().contains("404 Not Found"));
Review Comment:
> We could use more idiomatic tests
I agree. I will change the PR to follow this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "bvolpato (via GitHub)" <gi...@apache.org>.
bvolpato commented on code in PR #29262:
URL: https://github.com/apache/beam/pull/29262#discussion_r1382423428
##########
sdks/java/io/rrio/src/test/java/org/apache/beam/io/requestresponse/EchoHTTPCallerTestIT.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.io.requestresponse;
+
+import static org.apache.beam.sdk.io.common.IOITHelper.readIOTestPipelineOptions;
+import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import com.google.protobuf.ByteString;
+import java.net.URI;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoRequest;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoResponse;
+import org.apache.beam.testinfra.mockapis.echo.v1.EchoServiceGrpc;
+import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests for {@link EchoHTTPCallerTestIT} on a deployed {@link EchoServiceGrpc} instance's HTTP
+ * handler. See {@link EchoITOptions} for details on the required parameters and how to provide
+ * these for running integration tests.
+ */
+@RunWith(JUnit4.class)
+public class EchoHTTPCallerTestIT {
+
+ private static @MonotonicNonNull EchoITOptions options;
+ private static @MonotonicNonNull EchoHTTPCaller client;
+ private static final ByteString PAYLOAD = ByteString.copyFromUtf8("payload");
+
+ @BeforeClass
+ public static void setUp() throws UserCodeExecutionException {
+ options = readIOTestPipelineOptions(EchoITOptions.class);
+ if (options.getHttpEndpointAddress().isEmpty()) {
+ throw new RuntimeException(
+ "--httpEndpointAddress is missing. See " + EchoITOptions.class + "for details.");
+ }
+ client = EchoHTTPCaller.of(URI.create(options.getHttpEndpointAddress()));
+
+ EchoRequest request = createShouldExceedQuotaRequest();
+
+ // Quota allocation needs at least 1 so we invoke a few times to guarantee exceeding quota.
+ // We only throw exceptions that are not UserCodeQuotaException.
+ try {
+ EchoResponse ignored = client.call(request);
+ client.call(request);
+ client.call(request);
+ client.call(request);
+ } catch (UserCodeExecutionException ignored) {
+ }
+ }
+
+ @Test
+ public void givenValidRequest_receivesResponse() throws UserCodeExecutionException {
+ EchoRequest request = createShouldNeverExceedQuotaRequest();
+ EchoResponse response = client.call(request);
+ assertEquals(response.getId(), request.getId());
+ assertEquals(response.getPayload(), request.getPayload());
+ }
+
+ @Test
+ public void givenExceededQuota_shouldThrow() {
+ assertThrows(UserCodeQuotaException.class, () -> client.call(createShouldExceedQuotaRequest()));
+ }
+
+ @Test
+ public void givenNotFound_shouldThrow() {
+ UserCodeExecutionException error =
+ assertThrows(
+ UserCodeExecutionException.class,
+ () ->
+ client.call(
+ EchoRequest.newBuilder()
+ .setId("i-dont-exist-quota-id")
+ .setPayload(PAYLOAD)
+ .build()));
+
+ assertTrue(error.getMessage().contains("404 Not Found"));
Review Comment:
Just curious if we have/can use something like Mockito?
We could use more idiomatic tests, for example
```
assertThat(error.getMessage(), containsString("404 Not Found"));
```
It looks like the test is the same, but failure reports would be (just an example):
> String is 429 Too Many Requests, expected to contain 404 Not Found
Instead of:
> Expected true is false
##########
sdks/java/io/rrio/src/test/java/org/apache/beam/io/requestresponse/EchoHTTPCallerTestIT.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.io.requestresponse;
+
+import static org.apache.beam.sdk.io.common.IOITHelper.readIOTestPipelineOptions;
+import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import com.google.protobuf.ByteString;
+import java.net.URI;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoRequest;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoResponse;
+import org.apache.beam.testinfra.mockapis.echo.v1.EchoServiceGrpc;
+import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests for {@link EchoHTTPCallerTestIT} on a deployed {@link EchoServiceGrpc} instance's HTTP
+ * handler. See {@link EchoITOptions} for details on the required parameters and how to provide
+ * these for running integration tests.
+ */
+@RunWith(JUnit4.class)
+public class EchoHTTPCallerTestIT {
+
+ private static @MonotonicNonNull EchoITOptions options;
+ private static @MonotonicNonNull EchoHTTPCaller client;
+ private static final ByteString PAYLOAD = ByteString.copyFromUtf8("payload");
+
+ @BeforeClass
+ public static void setUp() throws UserCodeExecutionException {
+ options = readIOTestPipelineOptions(EchoITOptions.class);
+ if (options.getHttpEndpointAddress().isEmpty()) {
+ throw new RuntimeException(
+ "--httpEndpointAddress is missing. See " + EchoITOptions.class + "for details.");
+ }
+ client = EchoHTTPCaller.of(URI.create(options.getHttpEndpointAddress()));
+
+ EchoRequest request = createShouldExceedQuotaRequest();
+
+ // Quota allocation needs at least 1 so we invoke a few times to guarantee exceeding quota.
+ // We only throw exceptions that are not UserCodeQuotaException.
+ try {
+ EchoResponse ignored = client.call(request);
+ client.call(request);
+ client.call(request);
+ client.call(request);
+ } catch (UserCodeExecutionException ignored) {
+ }
Review Comment:
It feels like it's part of some initialization, but feels like we're testing some behavior here. Let's make sure that this is covered (with assertions) somewhere else
##########
sdks/java/io/rrio/build.gradle:
##########
@@ -34,15 +38,18 @@ dependencies {
testImplementation project(path: ":sdks:java:core", configuration: "shadowTest")
testImplementation project(path: ":sdks:java:io:common", configuration: "testRuntimeMigration")
+ testImplementation project(path: ":beam-test-infra-mock-apis")
+ // Vendered grpc library not fully compatible with proto autogenerated code
Review Comment:
nit typo Vendored
##########
sdks/java/io/rrio/src/test/java/org/apache/beam/io/requestresponse/EchoITOptions.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.io.requestresponse;
+
+import org.apache.beam.sdk.options.Default;
+import org.apache.beam.sdk.options.Description;
+import org.apache.beam.sdk.options.PipelineOptions;
+import org.apache.beam.testinfra.mockapis.echo.v1.EchoServiceGrpc;
+
+/**
+ * Shared options for running integration tests on a deployed {@link EchoServiceGrpc}. See <a
+ * href="https://github.com/apache/beam/tree/master/.test-infra/mock-apis#integration">https://github.com/apache/beam/tree/master/.test-infra/mock-apis#integration</a>
+ * for details on how to acquire values required by {@link EchoITOptions}.
+ *
+ * <p>To provide these values to your integration tests:
+ *
+ * <pre>
+ * ./gradlew :sdks:java:io:rrio:integrationTest -DintegrationTestPipelineOptions='[
+ * "--grpcEndpointAddress=",
+ * "--httpEndpointAddress="
+ * ]'
+ * </pre>
+ */
+public interface EchoITOptions extends PipelineOptions {
+ @Description("The gRPC address of the Echo API endpoint, typically of the form <host>:<port>.")
+ String getgRPCEndpointAddress();
Review Comment:
Can you test this explicitly to make sure this works as intended?
It feels to me that it should be `getGRPCEndpointAddress` to follow Java conventions, but I am not sure if the PipelineOptions is case sensitive.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "damondouglas (via GitHub)" <gi...@apache.org>.
damondouglas commented on code in PR #29262:
URL: https://github.com/apache/beam/pull/29262#discussion_r1383802980
##########
sdks/java/io/rrio/src/test/java/org/apache/beam/io/requestresponse/EchoITOptions.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.io.requestresponse;
+
+import org.apache.beam.sdk.options.Default;
+import org.apache.beam.sdk.options.Description;
+import org.apache.beam.sdk.options.PipelineOptions;
+import org.apache.beam.testinfra.mockapis.echo.v1.EchoServiceGrpc;
+
+/**
+ * Shared options for running integration tests on a deployed {@link EchoServiceGrpc}. See <a
+ * href="https://github.com/apache/beam/tree/master/.test-infra/mock-apis#integration">https://github.com/apache/beam/tree/master/.test-infra/mock-apis#integration</a>
+ * for details on how to acquire values required by {@link EchoITOptions}.
+ *
+ * <p>To provide these values to your integration tests:
+ *
+ * <pre>
+ * ./gradlew :sdks:java:io:rrio:integrationTest -DintegrationTestPipelineOptions='[
+ * "--grpcEndpointAddress=",
+ * "--httpEndpointAddress="
+ * ]'
+ * </pre>
+ */
+public interface EchoITOptions extends PipelineOptions {
+ @Description("The gRPC address of the Echo API endpoint, typically of the form <host>:<port>.")
+ String getgRPCEndpointAddress();
Review Comment:
I experimented with `getGRPCEndpointAddress` and it is case sensitive. Setting as `getgRPCEndpointAddress` allows the flag to look like this: `--gRPCEndpointAddress`. Using `getGRPCEndpointAddress` looks like this: `--GRPCEndpointAddress`. Given the canonical name is with the little 'g', I opted for `getgRPCEndpointAddress`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "damondouglas (via GitHub)" <gi...@apache.org>.
damondouglas commented on PR #29262:
URL: https://github.com/apache/beam/pull/29262#issuecomment-1796585007
Run Java PreCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "bvolpato (via GitHub)" <gi...@apache.org>.
bvolpato commented on code in PR #29262:
URL: https://github.com/apache/beam/pull/29262#discussion_r1384295296
##########
sdks/java/io/rrio/src/test/java/org/apache/beam/io/requestresponse/EchoHTTPCallerTestIT.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.io.requestresponse;
+
+import static org.apache.beam.sdk.io.common.IOITHelper.readIOTestPipelineOptions;
+import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import com.google.protobuf.ByteString;
+import java.net.URI;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoRequest;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoResponse;
+import org.apache.beam.testinfra.mockapis.echo.v1.EchoServiceGrpc;
+import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests for {@link EchoHTTPCallerTestIT} on a deployed {@link EchoServiceGrpc} instance's HTTP
+ * handler. See {@link EchoITOptions} for details on the required parameters and how to provide
+ * these for running integration tests.
+ */
+@RunWith(JUnit4.class)
+public class EchoHTTPCallerTestIT {
+
+ private static @MonotonicNonNull EchoITOptions options;
+ private static @MonotonicNonNull EchoHTTPCaller client;
+ private static final ByteString PAYLOAD = ByteString.copyFromUtf8("payload");
+
+ @BeforeClass
+ public static void setUp() throws UserCodeExecutionException {
+ options = readIOTestPipelineOptions(EchoITOptions.class);
+ if (options.getHttpEndpointAddress().isEmpty()) {
+ throw new RuntimeException(
+ "--httpEndpointAddress is missing. See " + EchoITOptions.class + "for details.");
+ }
+ client = EchoHTTPCaller.of(URI.create(options.getHttpEndpointAddress()));
+
+ EchoRequest request = createShouldExceedQuotaRequest();
+
+ // Quota allocation needs at least 1 so we invoke a few times to guarantee exceeding quota.
+ // We only throw exceptions that are not UserCodeQuotaException.
+ try {
+ EchoResponse ignored = client.call(request);
+ client.call(request);
+ client.call(request);
+ client.call(request);
+ } catch (UserCodeExecutionException ignored) {
+ }
+ }
+
+ @Test
+ public void givenValidRequest_receivesResponse() throws UserCodeExecutionException {
+ EchoRequest request = createShouldNeverExceedQuotaRequest();
+ EchoResponse response = client.call(request);
+ assertEquals(response.getId(), request.getId());
+ assertEquals(response.getPayload(), request.getPayload());
+ }
+
+ @Test
+ public void givenExceededQuota_shouldThrow() {
+ assertThrows(UserCodeQuotaException.class, () -> client.call(createShouldExceedQuotaRequest()));
+ }
+
+ @Test
+ public void givenNotFound_shouldThrow() {
+ UserCodeExecutionException error =
+ assertThrows(
+ UserCodeExecutionException.class,
+ () ->
+ client.call(
+ EchoRequest.newBuilder()
+ .setId("i-dont-exist-quota-id")
+ .setPayload(PAYLOAD)
+ .build()));
+
+ assertTrue(error.getMessage().contains("404 Not Found"));
Review Comment:
Sorry, I meant Hamcrest instead of Mockito.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "damondouglas (via GitHub)" <gi...@apache.org>.
damondouglas commented on code in PR #29262:
URL: https://github.com/apache/beam/pull/29262#discussion_r1383797998
##########
sdks/java/io/rrio/src/test/java/org/apache/beam/io/requestresponse/EchoHTTPCallerTestIT.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.io.requestresponse;
+
+import static org.apache.beam.sdk.io.common.IOITHelper.readIOTestPipelineOptions;
+import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import com.google.protobuf.ByteString;
+import java.net.URI;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoRequest;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoResponse;
+import org.apache.beam.testinfra.mockapis.echo.v1.EchoServiceGrpc;
+import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests for {@link EchoHTTPCallerTestIT} on a deployed {@link EchoServiceGrpc} instance's HTTP
+ * handler. See {@link EchoITOptions} for details on the required parameters and how to provide
+ * these for running integration tests.
+ */
+@RunWith(JUnit4.class)
+public class EchoHTTPCallerTestIT {
+
+ private static @MonotonicNonNull EchoITOptions options;
+ private static @MonotonicNonNull EchoHTTPCaller client;
+ private static final ByteString PAYLOAD = ByteString.copyFromUtf8("payload");
+
+ @BeforeClass
+ public static void setUp() throws UserCodeExecutionException {
+ options = readIOTestPipelineOptions(EchoITOptions.class);
+ if (options.getHttpEndpointAddress().isEmpty()) {
+ throw new RuntimeException(
+ "--httpEndpointAddress is missing. See " + EchoITOptions.class + "for details.");
+ }
+ client = EchoHTTPCaller.of(URI.create(options.getHttpEndpointAddress()));
+
+ EchoRequest request = createShouldExceedQuotaRequest();
+
+ // Quota allocation needs at least 1 so we invoke a few times to guarantee exceeding quota.
+ // We only throw exceptions that are not UserCodeQuotaException.
+ try {
+ EchoResponse ignored = client.call(request);
+ client.call(request);
+ client.call(request);
+ client.call(request);
+ } catch (UserCodeExecutionException ignored) {
+ }
+ }
+
+ @Test
+ public void givenValidRequest_receivesResponse() throws UserCodeExecutionException {
+ EchoRequest request = createShouldNeverExceedQuotaRequest();
+ EchoResponse response = client.call(request);
+ assertEquals(response.getId(), request.getId());
+ assertEquals(response.getPayload(), request.getPayload());
+ }
+
+ @Test
+ public void givenExceededQuota_shouldThrow() {
+ assertThrows(UserCodeQuotaException.class, () -> client.call(createShouldExceedQuotaRequest()));
+ }
+
+ @Test
+ public void givenNotFound_shouldThrow() {
+ UserCodeExecutionException error =
+ assertThrows(
+ UserCodeExecutionException.class,
+ () ->
+ client.call(
+ EchoRequest.newBuilder()
+ .setId("i-dont-exist-quota-id")
+ .setPayload(PAYLOAD)
+ .build()));
+
+ assertTrue(error.getMessage().contains("404 Not Found"));
Review Comment:
> We could use more idiomatic tests, for example `assertThat`
I just checked that in the JUnit library, assertThat is deprecated. I will keep the assertions the way they are and resolve this helpful comment. However, please re-open if you would like to continue discussing. Thank you, again!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #29262:
URL: https://github.com/apache/beam/pull/29262#issuecomment-1791567650
Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @kennknowles for label java.
R: @damccorm for label build.
R: @bvolpato for label io.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "damondouglas (via GitHub)" <gi...@apache.org>.
damondouglas commented on code in PR #29262:
URL: https://github.com/apache/beam/pull/29262#discussion_r1383729881
##########
sdks/java/io/rrio/src/test/java/org/apache/beam/io/requestresponse/EchoHTTPCallerTestIT.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.io.requestresponse;
+
+import static org.apache.beam.sdk.io.common.IOITHelper.readIOTestPipelineOptions;
+import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import com.google.protobuf.ByteString;
+import java.net.URI;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoRequest;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoResponse;
+import org.apache.beam.testinfra.mockapis.echo.v1.EchoServiceGrpc;
+import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests for {@link EchoHTTPCallerTestIT} on a deployed {@link EchoServiceGrpc} instance's HTTP
+ * handler. See {@link EchoITOptions} for details on the required parameters and how to provide
+ * these for running integration tests.
+ */
+@RunWith(JUnit4.class)
+public class EchoHTTPCallerTestIT {
+
+ private static @MonotonicNonNull EchoITOptions options;
+ private static @MonotonicNonNull EchoHTTPCaller client;
+ private static final ByteString PAYLOAD = ByteString.copyFromUtf8("payload");
+
+ @BeforeClass
+ public static void setUp() throws UserCodeExecutionException {
+ options = readIOTestPipelineOptions(EchoITOptions.class);
+ if (options.getHttpEndpointAddress().isEmpty()) {
+ throw new RuntimeException(
+ "--httpEndpointAddress is missing. See " + EchoITOptions.class + "for details.");
+ }
+ client = EchoHTTPCaller.of(URI.create(options.getHttpEndpointAddress()));
+
+ EchoRequest request = createShouldExceedQuotaRequest();
+
+ // Quota allocation needs at least 1 so we invoke a few times to guarantee exceeding quota.
+ // We only throw exceptions that are not UserCodeQuotaException.
+ try {
+ EchoResponse ignored = client.call(request);
+ client.call(request);
+ client.call(request);
+ client.call(request);
+ } catch (UserCodeExecutionException ignored) {
+ }
Review Comment:
@bvolpato Thank you for catching this. The challenge with building and deploying a real quota aware endpoint, the integration with which these tests validate, is that we need a value of at least 1. The allocated quota where we expect to exceed will be shared among many tests and across languages. This setup insures that the API is in the state where we can expect a quota exceeded error. There are tests in this file that detect errors in expected responses. I will update the comments with this prior explanation and resolve this comment. However, please re-open if you feel strongly that some further change is warranted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "damondouglas (via GitHub)" <gi...@apache.org>.
damondouglas commented on code in PR #29262:
URL: https://github.com/apache/beam/pull/29262#discussion_r1385282037
##########
sdks/java/io/rrio/src/test/java/org/apache/beam/io/requestresponse/EchoHTTPCallerTestIT.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.io.requestresponse;
+
+import static org.apache.beam.sdk.io.common.IOITHelper.readIOTestPipelineOptions;
+import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import com.google.protobuf.ByteString;
+import java.net.URI;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoRequest;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoResponse;
+import org.apache.beam.testinfra.mockapis.echo.v1.EchoServiceGrpc;
+import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests for {@link EchoHTTPCallerTestIT} on a deployed {@link EchoServiceGrpc} instance's HTTP
+ * handler. See {@link EchoITOptions} for details on the required parameters and how to provide
+ * these for running integration tests.
+ */
+@RunWith(JUnit4.class)
+public class EchoHTTPCallerTestIT {
+
+ private static @MonotonicNonNull EchoITOptions options;
+ private static @MonotonicNonNull EchoHTTPCaller client;
+ private static final ByteString PAYLOAD = ByteString.copyFromUtf8("payload");
+
+ @BeforeClass
+ public static void setUp() throws UserCodeExecutionException {
+ options = readIOTestPipelineOptions(EchoITOptions.class);
+ if (options.getHttpEndpointAddress().isEmpty()) {
+ throw new RuntimeException(
+ "--httpEndpointAddress is missing. See " + EchoITOptions.class + "for details.");
+ }
+ client = EchoHTTPCaller.of(URI.create(options.getHttpEndpointAddress()));
+
+ EchoRequest request = createShouldExceedQuotaRequest();
+
+ // Quota allocation needs at least 1 so we invoke a few times to guarantee exceeding quota.
+ // We only throw exceptions that are not UserCodeQuotaException.
+ try {
+ EchoResponse ignored = client.call(request);
+ client.call(request);
+ client.call(request);
+ client.call(request);
+ } catch (UserCodeExecutionException ignored) {
+ }
+ }
+
+ @Test
+ public void givenValidRequest_receivesResponse() throws UserCodeExecutionException {
+ EchoRequest request = createShouldNeverExceedQuotaRequest();
+ EchoResponse response = client.call(request);
+ assertEquals(response.getId(), request.getId());
+ assertEquals(response.getPayload(), request.getPayload());
+ }
+
+ @Test
+ public void givenExceededQuota_shouldThrow() {
+ assertThrows(UserCodeQuotaException.class, () -> client.call(createShouldExceedQuotaRequest()));
+ }
+
+ @Test
+ public void givenNotFound_shouldThrow() {
+ UserCodeExecutionException error =
+ assertThrows(
+ UserCodeExecutionException.class,
+ () ->
+ client.call(
+ EchoRequest.newBuilder()
+ .setId("i-dont-exist-quota-id")
+ .setPayload(PAYLOAD)
+ .build()));
+
+ assertTrue(error.getMessage().contains("404 Not Found"));
Review Comment:
I agree. Hamcrest seems much more readable. I will change over to using that. Thank you, Bruno.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "damondouglas (via GitHub)" <gi...@apache.org>.
damondouglas commented on code in PR #29262:
URL: https://github.com/apache/beam/pull/29262#discussion_r1383729881
##########
sdks/java/io/rrio/src/test/java/org/apache/beam/io/requestresponse/EchoHTTPCallerTestIT.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.io.requestresponse;
+
+import static org.apache.beam.sdk.io.common.IOITHelper.readIOTestPipelineOptions;
+import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import com.google.protobuf.ByteString;
+import java.net.URI;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoRequest;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoResponse;
+import org.apache.beam.testinfra.mockapis.echo.v1.EchoServiceGrpc;
+import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests for {@link EchoHTTPCallerTestIT} on a deployed {@link EchoServiceGrpc} instance's HTTP
+ * handler. See {@link EchoITOptions} for details on the required parameters and how to provide
+ * these for running integration tests.
+ */
+@RunWith(JUnit4.class)
+public class EchoHTTPCallerTestIT {
+
+ private static @MonotonicNonNull EchoITOptions options;
+ private static @MonotonicNonNull EchoHTTPCaller client;
+ private static final ByteString PAYLOAD = ByteString.copyFromUtf8("payload");
+
+ @BeforeClass
+ public static void setUp() throws UserCodeExecutionException {
+ options = readIOTestPipelineOptions(EchoITOptions.class);
+ if (options.getHttpEndpointAddress().isEmpty()) {
+ throw new RuntimeException(
+ "--httpEndpointAddress is missing. See " + EchoITOptions.class + "for details.");
+ }
+ client = EchoHTTPCaller.of(URI.create(options.getHttpEndpointAddress()));
+
+ EchoRequest request = createShouldExceedQuotaRequest();
+
+ // Quota allocation needs at least 1 so we invoke a few times to guarantee exceeding quota.
+ // We only throw exceptions that are not UserCodeQuotaException.
+ try {
+ EchoResponse ignored = client.call(request);
+ client.call(request);
+ client.call(request);
+ client.call(request);
+ } catch (UserCodeExecutionException ignored) {
+ }
Review Comment:
@bvolpato Thank you for catching this. The challenge with building and deploying a real quota aware endpoint, the integration with which these tests validate, is that we need a value of at least 1. The allocated quota where we expect to exceed will be shared among many tests and across languages. This setup ensures that the API is in the state where we can expect a quota exceeded error. There are tests in this file that detect errors in expected responses. I will update the comments with this prior explanation and resolve this comment. However, please re-open if you feel strongly that some further change is warranted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "damondouglas (via GitHub)" <gi...@apache.org>.
damondouglas commented on code in PR #29262:
URL: https://github.com/apache/beam/pull/29262#discussion_r1383803615
##########
sdks/java/io/rrio/src/test/java/org/apache/beam/io/requestresponse/EchoITOptions.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.io.requestresponse;
+
+import org.apache.beam.sdk.options.Default;
+import org.apache.beam.sdk.options.Description;
+import org.apache.beam.sdk.options.PipelineOptions;
+import org.apache.beam.testinfra.mockapis.echo.v1.EchoServiceGrpc;
+
+/**
+ * Shared options for running integration tests on a deployed {@link EchoServiceGrpc}. See <a
+ * href="https://github.com/apache/beam/tree/master/.test-infra/mock-apis#integration">https://github.com/apache/beam/tree/master/.test-infra/mock-apis#integration</a>
+ * for details on how to acquire values required by {@link EchoITOptions}.
+ *
+ * <p>To provide these values to your integration tests:
+ *
+ * <pre>
+ * ./gradlew :sdks:java:io:rrio:integrationTest -DintegrationTestPipelineOptions='[
+ * "--grpcEndpointAddress=",
+ * "--httpEndpointAddress="
+ * ]'
+ * </pre>
+ */
+public interface EchoITOptions extends PipelineOptions {
+ @Description("The gRPC address of the Echo API endpoint, typically of the form <host>:<port>.")
+ String getgRPCEndpointAddress();
Review Comment:
I will go ahead and resolve this but please re-open if you feel more discussion is warranted. Thank you, again for reviewing!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "damondouglas (via GitHub)" <gi...@apache.org>.
damondouglas commented on PR #29262:
URL: https://github.com/apache/beam/pull/29262#issuecomment-1791566074
assign set of reviewers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "damondouglas (via GitHub)" <gi...@apache.org>.
damondouglas commented on code in PR #29262:
URL: https://github.com/apache/beam/pull/29262#discussion_r1383797003
##########
sdks/java/io/rrio/src/test/java/org/apache/beam/io/requestresponse/EchoHTTPCallerTestIT.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.io.requestresponse;
+
+import static org.apache.beam.sdk.io.common.IOITHelper.readIOTestPipelineOptions;
+import static org.apache.beam.sdk.util.Preconditions.checkStateNotNull;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import com.google.protobuf.ByteString;
+import java.net.URI;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoRequest;
+import org.apache.beam.testinfra.mockapis.echo.v1.Echo.EchoResponse;
+import org.apache.beam.testinfra.mockapis.echo.v1.EchoServiceGrpc;
+import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests for {@link EchoHTTPCallerTestIT} on a deployed {@link EchoServiceGrpc} instance's HTTP
+ * handler. See {@link EchoITOptions} for details on the required parameters and how to provide
+ * these for running integration tests.
+ */
+@RunWith(JUnit4.class)
+public class EchoHTTPCallerTestIT {
+
+ private static @MonotonicNonNull EchoITOptions options;
+ private static @MonotonicNonNull EchoHTTPCaller client;
+ private static final ByteString PAYLOAD = ByteString.copyFromUtf8("payload");
+
+ @BeforeClass
+ public static void setUp() throws UserCodeExecutionException {
+ options = readIOTestPipelineOptions(EchoITOptions.class);
+ if (options.getHttpEndpointAddress().isEmpty()) {
+ throw new RuntimeException(
+ "--httpEndpointAddress is missing. See " + EchoITOptions.class + "for details.");
+ }
+ client = EchoHTTPCaller.of(URI.create(options.getHttpEndpointAddress()));
+
+ EchoRequest request = createShouldExceedQuotaRequest();
+
+ // Quota allocation needs at least 1 so we invoke a few times to guarantee exceeding quota.
+ // We only throw exceptions that are not UserCodeQuotaException.
+ try {
+ EchoResponse ignored = client.call(request);
+ client.call(request);
+ client.call(request);
+ client.call(request);
+ } catch (UserCodeExecutionException ignored) {
+ }
+ }
+
+ @Test
+ public void givenValidRequest_receivesResponse() throws UserCodeExecutionException {
+ EchoRequest request = createShouldNeverExceedQuotaRequest();
+ EchoResponse response = client.call(request);
+ assertEquals(response.getId(), request.getId());
+ assertEquals(response.getPayload(), request.getPayload());
+ }
+
+ @Test
+ public void givenExceededQuota_shouldThrow() {
+ assertThrows(UserCodeQuotaException.class, () -> client.call(createShouldExceedQuotaRequest()));
+ }
+
+ @Test
+ public void givenNotFound_shouldThrow() {
+ UserCodeExecutionException error =
+ assertThrows(
+ UserCodeExecutionException.class,
+ () ->
+ client.call(
+ EchoRequest.newBuilder()
+ .setId("i-dont-exist-quota-id")
+ .setPayload(PAYLOAD)
+ .build()));
+
+ assertTrue(error.getMessage().contains("404 Not Found"));
Review Comment:
> Just curious if we have/can use something like Mockito?
There are mocking tests in a separate PR that are validating catching specific errors and mock the Caller and SetupTeardown interfaces for this purpose. Tests in this PR validate the integration of the Caller and SetupTeardown implementation and a real API built and deployed in a cloud environment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "damondouglas (via GitHub)" <gi...@apache.org>.
damondouglas merged PR #29262:
URL: https://github.com/apache/beam/pull/29262
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [RRIO] [Test] Create test Caller and SetupTeardown [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #29262:
URL: https://github.com/apache/beam/pull/29262#issuecomment-1791501800
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org