[
https://issues.apache.org/jira/browse/BEAM-4326?focusedWorklogId=109713&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-109713
]
ASF GitHub Bot logged work on BEAM-4326:
----------------------------------------
Author: ASF GitHub Bot
Created on: 07/Jun/18 14:54
Start Date: 07/Jun/18 14:54
Worklog Time Spent: 10m
Work Description: iemejia closed pull request #5542: [BEAM-4326] Enforce
ErrorProne analysis in the fn-execution project
URL: https://github.com/apache/beam/pull/5542
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/sdks/java/fn-execution/build.gradle
b/sdks/java/fn-execution/build.gradle
index cf175a78f17..61c164681ac 100644
--- a/sdks/java/fn-execution/build.gradle
+++ b/sdks/java/fn-execution/build.gradle
@@ -17,7 +17,7 @@
*/
apply from: project(":").file("build_rules.gradle")
-applyJavaNature()
+applyJavaNature(failOnWarning: true)
description = "Apache Beam :: SDKs :: Java :: Fn Execution"
ext.summary = """Contains code shared across the Beam Java SDK Harness and
Java Runners to execute using
@@ -27,6 +27,7 @@ the Beam Portability Framework."""
dependencies {
compile library.java.guava
compile library.java.findbugs_jsr305
+ compileOnly library.java.findbugs_annotations
shadow project(path: ":beam-model-pipeline", configuration: "shadow")
shadow project(path: ":beam-model-fn-execution", configuration: "shadow")
shadow project(path: ":beam-sdks-java-core", configuration: "shadow")
@@ -41,4 +42,5 @@ dependencies {
testCompile library.java.hamcrest_library
testCompile library.java.mockito_core
testCompile library.java.slf4j_jdk14
+ testCompileOnly library.java.findbugs_annotations
}
diff --git
a/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/data/BeamFnDataGrpcMultiplexerTest.java
b/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/data/BeamFnDataGrpcMultiplexerTest.java
index b1abb07434d..b39a1c9b217 100644
---
a/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/data/BeamFnDataGrpcMultiplexerTest.java
+++
b/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/data/BeamFnDataGrpcMultiplexerTest.java
@@ -80,7 +80,7 @@ public void testInboundObserverBlocksTillConsumerConnects()
throws Exception {
// Purposefully sleep to simulate a delay in a consumer connecting.
Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS);
multiplexer.registerConsumer(OUTPUT_LOCATION, inboundValues::add);
- });
+ }).get();
multiplexer.getInboundObserver().onNext(ELEMENTS);
assertTrue(multiplexer.hasConsumer(OUTPUT_LOCATION));
// Ensure that when we see a terminal Elements object, we remove the
consumer
diff --git
a/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/data/CompletableFutureInboundDataClientTest.java
b/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/data/CompletableFutureInboundDataClientTest.java
index 277ab56ee91..f897d72ba9f 100644
---
a/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/data/CompletableFutureInboundDataClientTest.java
+++
b/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/data/CompletableFutureInboundDataClientTest.java
@@ -21,6 +21,7 @@
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.isA;
import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
import java.util.concurrent.CancellationException;
import java.util.concurrent.CompletableFuture;
@@ -100,6 +101,7 @@ public void testCompleteMultithreaded() throws Exception {
try {
waitingFuture.get(50, TimeUnit.MILLISECONDS);
+ fail();
} catch (TimeoutException expected) {
// This should time out, as the client should never complete without
external completion
}
diff --git
a/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/stream/AdvancingPhaserTest.java
b/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/stream/AdvancingPhaserTest.java
index a112a165f1d..ad2ed52870b 100644
---
a/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/stream/AdvancingPhaserTest.java
+++
b/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/stream/AdvancingPhaserTest.java
@@ -37,7 +37,7 @@ public void testAdvancement() throws Exception {
final AdvancingPhaser phaser = new AdvancingPhaser(1);
int currentPhase = phaser.getPhase();
ExecutorService service = Executors.newSingleThreadExecutor();
- service.submit((Runnable) phaser::arrive);
+ service.submit((Runnable) phaser::arrive).get();
phaser.awaitAdvance(currentPhase);
assertFalse(phaser.isTerminated());
service.shutdown();
diff --git
a/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/test/TestExecutorsTest.java
b/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/test/TestExecutorsTest.java
index 7c76a072115..204593fd916 100644
---
a/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/test/TestExecutorsTest.java
+++
b/sdks/java/fn-execution/src/test/java/org/apache/beam/sdk/fn/test/TestExecutorsTest.java
@@ -45,7 +45,7 @@ public void testSuccessfulTermination() throws Throwable {
new Statement() {
@Override
public void evaluate() throws Throwable {
- testService.submit(() -> taskRan.set(true));
+ testService.submit(() -> taskRan.set(true)).get();
}
},
null)
@@ -55,6 +55,10 @@ public void evaluate() throws Throwable {
}
@Test
+ // FutureReturnValueIgnored suppression is safe because testService is
+ // expected to *not* shutdown cleanly on the task it was given to execute.
+ // If we try to obtain the result of the future the test will timeout.
+ @SuppressWarnings("FutureReturnValueIgnored")
public void testTaskBlocksForeverCausesFailure() throws Throwable {
ExecutorService service = Executors.newSingleThreadExecutor();
final TestExecutorService testService = TestExecutors.from(service);
@@ -115,6 +119,10 @@ public void evaluate() throws Throwable {
}
@Test
+ // FutureReturnValueIgnored suppression is safe because testService is
+ // expected to *not* shutdown cleanly on the task it was given to execute.
+ // If we try to obtain the result of the future the test will timeout.
+ @SuppressWarnings("FutureReturnValueIgnored")
public void
testStatementFailurePropagatedWhenExecutorServiceFailingToTerminate()
throws Throwable {
ExecutorService service = Executors.newSingleThreadExecutor();
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 109713)
Time Spent: 1h 40m (was: 1.5h)
> Enforce ErrorProne analysis in the fn-execution project
> -------------------------------------------------------
>
> Key: BEAM-4326
> URL: https://issues.apache.org/jira/browse/BEAM-4326
> Project: Beam
> Issue Type: Improvement
> Components: sdk-java-harness
> Reporter: Scott Wegner
> Assignee: Cade Markegard
> Priority: Minor
> Labels: errorprone, starter
> Fix For: 2.6.0
>
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> Java ErrorProne static analysis was [recently
> enabled|https://github.com/apache/beam/pull/5161] in the Gradle build
> process, but only as warnings. ErrorProne errors are generally useful and
> easy to fix. Some work was done to [make sdks-java-core
> ErrorProne-clean|https://github.com/apache/beam/pull/5319] and add
> enforcement. This task is clean ErrorProne warnings and add enforcement in
> {{beam-sdks-java-fn-execution}}. Additional context discussed on the [dev
> list|https://lists.apache.org/thread.html/95aae2785c3cd728c2d3378cbdff2a7ba19caffcd4faa2049d2e2f46@%3Cdev.beam.apache.org%3E].
> Fixing this issue will involve:
> # Follow instructions in the [Contribution
> Guide|https://beam.apache.org/contribute/] to set up a {{beam}} development
> environment.
> # Run the following command to compile and run ErrorProne analysis on the
> project: {{./gradlew :beam-sdks-java-fn-execution:assemble}}
> # Fix each ErrorProne warning from the {{sdks/java/fn-execution}} project.
> # In {{sdks/java/fn-execution/build.gradle}}, add {{failOnWarning: true}} to
> the call the {{applyJavaNature()}}
> ([example|https://github.com/apache/beam/pull/5319/files#diff-9390c20635aed5f42f83b97506a87333R20]).
> This starter issue is sponsored by [~swegner]. Feel free to [reach
> out|https://beam.apache.org/community/contact-us/] with questions or code
> review:
> * JIRA: [~swegner]
> * GitHub: [@swegner|https://github.com/swegner]
> * Slack: [@Scott Wegner|https://s.apache.org/beam-slack-channel]
> * Email: swegner at google dot com
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
