[
https://issues.apache.org/jira/browse/BEAM-4308?focusedWorklogId=105951&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-105951
]
ASF GitHub Bot logged work on BEAM-4308:
----------------------------------------
Author: ASF GitHub Bot
Created on: 25/May/18 16:47
Start Date: 25/May/18 16:47
Worklog Time Spent: 10m
Work Description: kennknowles closed pull request #5484: [BEAM-4308]
Enforce ErrorProne analysis in runners-core-java
URL: https://github.com/apache/beam/pull/5484
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/runners/core-java/build.gradle b/runners/core-java/build.gradle
index 3d77c40c103..95550a8ff80 100644
--- a/runners/core-java/build.gradle
+++ b/runners/core-java/build.gradle
@@ -17,7 +17,7 @@
*/
apply from: project(":").file("build_rules.gradle")
-applyJavaNature()
+applyJavaNature(failOnWarning: true)
description = "Apache Beam :: Runners :: Core Java"
ext.summary = "Beam Runners Core provides utilities to aid runner authors."
@@ -42,6 +42,7 @@ dependencies {
shadow library.java.grpc_stub
shadow library.java.joda_time
shadow library.java.slf4j_api
+ testCompileOnly library.java.findbugs_annotations
shadowTest project(path: ":beam-sdks-java-core", configuration: "shadowTest")
shadowTest library.java.hamcrest_core
shadowTest library.java.mockito_core
diff --git
a/runners/core-java/src/main/java/org/apache/beam/runners/core/StateTags.java
b/runners/core-java/src/main/java/org/apache/beam/runners/core/StateTags.java
index da94ef28971..615d0505593 100644
---
a/runners/core-java/src/main/java/org/apache/beam/runners/core/StateTags.java
+++
b/runners/core-java/src/main/java/org/apache/beam/runners/core/StateTags.java
@@ -125,7 +125,7 @@ public WatermarkHoldState bindWatermark(
SYSTEM('s'),
USER('u');
- private char prefix;
+ private final char prefix;
StateKind(char prefix) {
this.prefix = prefix;
diff --git
a/runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/CounterCell.java
b/runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/CounterCell.java
index 886d681ef59..ad468424fd3 100644
---
a/runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/CounterCell.java
+++
b/runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/CounterCell.java
@@ -61,14 +61,17 @@ public void inc(long n) {
dirty.afterModification();
}
+ @Override
public void inc() {
inc(1);
}
+ @Override
public void dec() {
inc(-1);
}
+ @Override
public void dec(long n) {
inc(-1 * n);
}
diff --git
a/runners/core-java/src/main/java/org/apache/beam/runners/core/triggers/TriggerStateMachine.java
b/runners/core-java/src/main/java/org/apache/beam/runners/core/triggers/TriggerStateMachine.java
index caeae18c48a..ed0b795238f 100644
---
a/runners/core-java/src/main/java/org/apache/beam/runners/core/triggers/TriggerStateMachine.java
+++
b/runners/core-java/src/main/java/org/apache/beam/runners/core/triggers/TriggerStateMachine.java
@@ -180,7 +180,7 @@
* <p>Used directly in {@link TriggerStateMachine#shouldFire} and {@link
* TriggerStateMachine#clear}, and extended with additional information in
other methods.
*/
- public abstract class TriggerContext {
+ public abstract static class TriggerContext {
/** Returns the interface for accessing trigger info. */
public abstract TriggerInfo trigger();
@@ -216,7 +216,7 @@
* Extended {@link TriggerContext} containing information accessible to the
{@link #onElement}
* operational hook.
*/
- public abstract class OnElementContext extends TriggerContext {
+ public abstract static class OnElementContext extends TriggerContext {
/** The event timestamp of the element currently being processed. */
public abstract Instant eventTimestamp();
@@ -242,7 +242,7 @@
* Extended {@link TriggerContext} containing information accessible to the
{@link #onMerge}
* operational hook.
*/
- public abstract class OnMergeContext extends TriggerContext {
+ public abstract static class OnMergeContext extends TriggerContext {
/**
* Sets a timer to fire when the watermark or processing time is beyond
the given timestamp.
* Timers are not guaranteed to fire immediately, but will be delivered at
some time afterwards.
diff --git
a/runners/core-java/src/main/java/org/apache/beam/runners/core/triggers/TriggerStateMachineContextFactory.java
b/runners/core-java/src/main/java/org/apache/beam/runners/core/triggers/TriggerStateMachineContextFactory.java
index cfe2d319cf7..83ee8c64fe6 100644
---
a/runners/core-java/src/main/java/org/apache/beam/runners/core/triggers/TriggerStateMachineContextFactory.java
+++
b/runners/core-java/src/main/java/org/apache/beam/runners/core/triggers/TriggerStateMachineContextFactory.java
@@ -301,7 +301,6 @@ private TriggerContextImpl(
Timers timers,
ExecutableTriggerStateMachine trigger,
FinishedTriggers finishedSet) {
- trigger.getSpec().super();
this.window = window;
this.state = new StateAccessorImpl(window, trigger);
this.timers = new TriggerTimers(window, timers);
@@ -365,7 +364,6 @@ private OnElementContextImpl(
ExecutableTriggerStateMachine trigger,
FinishedTriggers finishedSet,
Instant eventTimestamp) {
- trigger.getSpec().super();
this.window = window;
this.state = new StateAccessorImpl(window, trigger);
this.timers = new TriggerTimers(window, timers);
@@ -442,7 +440,6 @@ private OnMergeContextImpl(
ExecutableTriggerStateMachine trigger,
FinishedTriggers finishedSet,
Map<W, FinishedTriggers> finishedSets) {
- trigger.getSpec().super();
this.mergingWindows = finishedSets.keySet();
this.window = window;
this.state = new MergingStateAccessorImpl(trigger, mergingWindows,
window);
diff --git
a/runners/core-java/src/test/java/org/apache/beam/runners/core/ReduceFnRunnerTest.java
b/runners/core-java/src/test/java/org/apache/beam/runners/core/ReduceFnRunnerTest.java
index 25a80bfead0..28ff92ab601 100644
---
a/runners/core-java/src/test/java/org/apache/beam/runners/core/ReduceFnRunnerTest.java
+++
b/runners/core-java/src/test/java/org/apache/beam/runners/core/ReduceFnRunnerTest.java
@@ -39,8 +39,8 @@
import static org.mockito.Mockito.withSettings;
import com.google.common.collect.Iterables;
+import java.util.ArrayList;
import java.util.Arrays;
-import java.util.LinkedList;
import java.util.List;
import java.util.Random;
import java.util.concurrent.ThreadLocalRandom;
@@ -142,7 +142,7 @@ private void injectElements(
ReduceFnTester<Integer, ?, IntervalWindow> tester, Iterable<Integer>
values)
throws Exception {
doNothing().when(mockTriggerStateMachine).onElement(anyElementContext());
- List<TimestampedValue<Integer>> timestampedValues = new LinkedList<>();
+ List<TimestampedValue<Integer>> timestampedValues = new ArrayList<>();
for (int value : values) {
timestampedValues.add(TimestampedValue.of(value, new Instant(value)));
}
@@ -403,7 +403,6 @@ public void testOnlyOneOnTimePane() throws Exception {
@Test
public void testOnElementCombiningDiscarding() throws Exception {
// Test basic execution of a trigger using a non-combining window set and
discarding mode.
-
WindowingStrategy<?, IntervalWindow> strategy =
WindowingStrategy.of((WindowFn<?, IntervalWindow>)
FixedWindows.of(Duration.millis(10)))
.withTimestampCombiner(TimestampCombiner.EARLIEST)
@@ -513,7 +512,6 @@ public void testCombiningAccumulatingProcessingTime()
throws Exception {
*/
@Test
public void testFixedWindowEndOfTimeGarbageCollection() throws Exception {
-
Duration allowedLateness = Duration.standardDays(365);
Duration windowSize = Duration.millis(10);
WindowFn<Object, IntervalWindow> windowFn = FixedWindows.of(windowSize);
@@ -639,7 +637,6 @@ public void testCombiningAccumulatingEventTime() throws
Exception {
@Test
public void testOnElementCombiningAccumulating() throws Exception {
// Test basic execution of a trigger using a non-combining window set and
accumulating mode.
-
WindowingStrategy<?, IntervalWindow> strategy =
WindowingStrategy.of((WindowFn<?, IntervalWindow>)
FixedWindows.of(Duration.millis(10)))
.withTimestampCombiner(TimestampCombiner.EARLIEST)
@@ -1035,7 +1032,7 @@ public void testMergingWatermarkHoldAndLateDataFuzz()
throws Exception {
assertEquals(null, tester.getOutputWatermark());
// All on time data, verify watermark hold.
- List<Integer> times = new LinkedList<>();
+ List<Integer> times = new ArrayList<>();
int numTs = 3 + r.nextInt(100);
int maxTs = 1 + r.nextInt(400);
@@ -1457,7 +1454,7 @@ public void testMergeBeforeFinalizing() throws Exception {
ReduceFnTester<Integer, Iterable<Integer>, IntervalWindow> tester =
ReduceFnTester.nonCombining(Sessions.withGapDuration(Duration.millis(10)),
mockTriggerStateMachine,
- AccumulationMode.DISCARDING_FIRED_PANES, Duration.millis(0),
+ AccumulationMode.DISCARDING_FIRED_PANES, Duration.ZERO,
ClosingBehavior.FIRE_IF_NON_EMPTY);
// All on time data, verify watermark hold.
@@ -1863,7 +1860,6 @@ public void testIdempotentEmptyPanesAccumulating() throws
Exception {
*/
@Test
public void testEmptyOnTimeFromOrFinally() throws Exception {
-
WindowingStrategy<?, IntervalWindow> strategy =
WindowingStrategy.of((WindowFn<?, IntervalWindow>)
FixedWindows.of(Duration.millis(10)))
.withTimestampCombiner(TimestampCombiner.EARLIEST)
@@ -1916,7 +1912,6 @@ public void testEmptyOnTimeFromOrFinally() throws
Exception {
*/
@Test
public void testEmptyOnTimeWithOnTimeBehaviorFireIfNonEmpty() throws
Exception {
-
WindowingStrategy<?, IntervalWindow> strategy =
WindowingStrategy.of((WindowFn<?, IntervalWindow>)
FixedWindows.of(Duration.millis(10)))
.withTimestampCombiner(TimestampCombiner.EARLIEST)
@@ -1977,14 +1972,13 @@ public void
testEmptyOnTimeWithOnTimeBehaviorFireIfNonEmpty() throws Exception {
*/
@Test
public void testEmptyOnTimeWithOnTimeBehaviorBackwardCompatibility() throws
Exception {
-
WindowingStrategy<?, IntervalWindow> strategy =
WindowingStrategy.of((WindowFn<?, IntervalWindow>)
FixedWindows.of(Duration.millis(10)))
.withTimestampCombiner(TimestampCombiner.EARLIEST)
.withTrigger(AfterWatermark.pastEndOfWindow()
.withEarlyFirings(AfterPane.elementCountAtLeast(1)))
.withMode(AccumulationMode.ACCUMULATING_FIRED_PANES)
- .withAllowedLateness(Duration.millis(0))
+ .withAllowedLateness(Duration.ZERO)
.withClosingBehavior(ClosingBehavior.FIRE_IF_NON_EMPTY);
ReduceFnTester<Integer, Integer, IntervalWindow> tester =
@@ -2016,7 +2010,6 @@ public void
testEmptyOnTimeWithOnTimeBehaviorBackwardCompatibility() throws Exce
*/
@Test
public void testEmptyOnTimeWithOnTimeBehaviorFireIfNonEmptyAndLateData()
throws Exception {
-
WindowingStrategy<?, IntervalWindow> strategy =
WindowingStrategy.of((WindowFn<?, IntervalWindow>)
FixedWindows.of(Duration.millis(10)))
.withTimestampCombiner(TimestampCombiner.EARLIEST)
@@ -2078,7 +2071,6 @@ public void
testEmptyOnTimeWithOnTimeBehaviorFireIfNonEmptyAndLateData() throws
*/
@Test
public void testProcessingTime() throws Exception {
-
WindowingStrategy<?, IntervalWindow> strategy =
WindowingStrategy.of((WindowFn<?, IntervalWindow>)
FixedWindows.of(Duration.millis(10)))
.withTimestampCombiner(TimestampCombiner.EARLIEST)
diff --git
a/runners/core-java/src/test/java/org/apache/beam/runners/core/StateInternalsTest.java
b/runners/core-java/src/test/java/org/apache/beam/runners/core/StateInternalsTest.java
index 79c972df997..79da4b446f4 100644
---
a/runners/core-java/src/test/java/org/apache/beam/runners/core/StateInternalsTest.java
+++
b/runners/core-java/src/test/java/org/apache/beam/runners/core/StateInternalsTest.java
@@ -299,27 +299,34 @@ private MapEntry(K key, V value) {
return new MapEntry<>(k, v);
}
+ @Override
public final K getKey() {
return key;
}
+
+ @Override
public final V getValue() {
return value;
}
+ @Override
public final String toString() {
return key + "=" + value;
}
+ @Override
public final int hashCode() {
return Objects.hashCode(key) ^ Objects.hashCode(value);
}
+ @Override
public final V setValue(V newValue) {
V oldValue = value;
value = newValue;
return oldValue;
}
+ @Override
public final boolean equals(Object o) {
if (o == this) {
return true;
diff --git
a/runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/MetricsContainerImplTest.java
b/runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/MetricsContainerImplTest.java
index ab4b709f235..baad9b00eab 100644
---
a/runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/MetricsContainerImplTest.java
+++
b/runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/MetricsContainerImplTest.java
@@ -95,7 +95,7 @@ public void testCounterCumulatives() {
metricUpdate("name2", 4L)));
CounterCell readC1 = container.tryGetCounter(MetricName.named("ns",
"name1"));
- assertEquals((long) readC1.getCumulative(), 13L);
+ assertEquals(13L, (long) readC1.getCumulative());
}
@Test
diff --git
a/runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/MetricsContainerStepMapTest.java
b/runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/MetricsContainerStepMapTest.java
index c8b172b0e8d..d6536923bcd 100644
---
a/runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/MetricsContainerStepMapTest.java
+++
b/runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/MetricsContainerStepMapTest.java
@@ -41,11 +41,14 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Tests for {@link MetricsContainerStepMap}.
*/
public class MetricsContainerStepMapTest {
+ private static final Logger LOG =
LoggerFactory.getLogger(MetricsContainerStepMapTest.class);
private static final String NAMESPACE =
MetricsContainerStepMapTest.class.getName();
private static final String STEP1 = "myStep1";
@@ -73,7 +76,7 @@
distribution.update(VALUE * 2);
gauge.set(VALUE);
} catch (IOException e) {
- e.printStackTrace();
+ LOG.error(e.getMessage(), e);
}
}
diff --git
a/runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/MetricsPusherTest.java
b/runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/MetricsPusherTest.java
index 2a6c7f3ffdf..b588fdcf0f7 100644
---
a/runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/MetricsPusherTest.java
+++
b/runners/core-java/src/test/java/org/apache/beam/runners/core/metrics/MetricsPusherTest.java
@@ -38,10 +38,13 @@
import org.junit.experimental.categories.Category;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/** A test that verifies that metrics push system works. */
@RunWith(JUnit4.class)
public class MetricsPusherTest {
+ private static final Logger LOG =
LoggerFactory.getLogger(MetricsPusherTest.class);
private static final long NUM_ELEMENTS = 1000L;
@Rule public final TestPipeline pipeline = TestPipeline.create();
@@ -76,7 +79,7 @@ public void processElement(ProcessContext context) {
counter.inc();
context.output(context.element());
} catch (Exception e) {
- e.printStackTrace();
+ LOG.error(e.getMessage(), e);
}
}
}
----------------------------------------------------------------
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: 105951)
Time Spent: 0.5h (was: 20m)
> Enforce ErrorProne analysis in runners-core project
> ---------------------------------------------------
>
> Key: BEAM-4308
> URL: https://issues.apache.org/jira/browse/BEAM-4308
> Project: Beam
> Issue Type: Improvement
> Components: runner-core
> Reporter: Scott Wegner
> Assignee: Ismaël Mejía
> Priority: Minor
> Labels: errorprone, starter
> Time Spent: 0.5h
> 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-runners-core-java}}. 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-runners-core-java:assemble}}
> # Fix each ErrorProne warning from the {{runners/core}} project.
> # In {{runners/core/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)
