Change in bazel[master]: Remove joda-time dependency from the java test runner

[Yunchi Luo (Gerrit)]

Yunchi Luo has uploaded a new change for review. (
https://bazel-review.googlesource.com/5550 )

Change subject: Remove joda-time dependency from the java test runner
......................................................................

Remove joda-time dependency from the java test runner

Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
---
M src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/AntXmlResultWriter.java
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/BUILD
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestCaseNode.java
A
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestSuiteNode.java
A src/java_tools/junitrunner/javatests/BUILD
A
src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/model/AllTests.java
A
src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/model/TestIntervalTest.java
11 files changed, 146 insertions(+), 29 deletions(-)



diff --git
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
index baa4ee8..327e311 100644
--- a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
+++ b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
@@ -20,7 +20,6 @@
"//src/java_tools/junitrunner/java/com/google/testing/junit/runner/model",
"//third_party:dagger",
"//third_party:guava",
- "//third_party:joda_time",
"//third_party:junit4",
],
)
diff --git
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java
index c9c40ee..846751c 100644
---
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java
+++
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java
@@ -29,11 +29,10 @@
import dagger.Module;
import dagger.Provides;

-import org.joda.time.DateTime;
-import org.joda.time.format.DateTimeFormat;
-import org.joda.time.format.DateTimeFormatter;
-
import java.io.PrintStream;
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.util.Date;
import java.util.concurrent.TimeUnit;

import javax.inject.Singleton;
@@ -97,9 +96,9 @@

printStackTracesIfJvmExitHangs(stderr);

- DateTimeFormatter formatter = DateTimeFormat.forPattern("yyyy-MM-dd
HH:mm:ss");
- DateTime shutdownTime = new DateTime();
- String formattedShutdownTime = formatter.print(shutdownTime);
+ DateFormat format = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
+ Date shutdownTime = new Date();
+ String formattedShutdownTime = format.format(shutdownTime);
System.err.printf("-- JVM shutdown starting at %s --%n%n",
formattedShutdownTime);
System.exit(exitCode);
}
diff --git
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/AntXmlResultWriter.java
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/AntXmlResultWriter.java
index a9174ce..8ee4ba3 100644
---
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/AntXmlResultWriter.java
+++
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/AntXmlResultWriter.java
@@ -22,7 +22,6 @@
import java.util.Map.Entry;
import javax.annotation.Nullable;
import javax.inject.Inject;
-import org.joda.time.Interval;

/**
* Writes the JUnit test nodes and their results into Ant-JUnit XML.
Ant-JUnit XML is not a
@@ -146,13 +145,13 @@
writer.writeAttribute(JUNIT_ATTR_TESTSUITE_ID, this.testSuiteId++);
}

- private static String getFormattedRunTime(@Nullable Interval
runTimeInterval) {
+ private static String getFormattedRunTime(@Nullable TestInterval
runTimeInterval) {
return runTimeInterval == null ? "0.0"
: String.valueOf(runTimeInterval.toDurationMillis() / 1000.0D);
}

- private static String getFormattedTimestamp(@Nullable Interval
runTimeInterval) {
- return runTimeInterval == null ? "" :
runTimeInterval.getStart().toString();
+ private static String getFormattedTimestamp(@Nullable TestInterval
runTimeInterval) {
+ return runTimeInterval == null ? "" :
runTimeInterval.startInstantToString();
}

private void writeTestCase(XmlWriter writer, TestResult result,
diff --git
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/BUILD
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/BUILD
index 18e37d1..44bd378 100644
---
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/BUILD
+++
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/BUILD
@@ -8,7 +8,6 @@
"//src/java_tools/junitrunner/java/com/google/testing/junit/runner/sharding",
"//src/java_tools/junitrunner/java/com/google/testing/junit/runner/util",
"//third_party:guava",
- "//third_party:joda_time",
"//third_party:jsr305",
"//third_party:jsr330_inject",
"//third_party:junit4",
diff --git
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestCaseNode.java
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestCaseNode.java
index bfa57ae..8e97d58 100644
---
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestCaseNode.java
+++
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestCaseNode.java
@@ -31,7 +31,6 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedQueue;
import javax.annotation.Nullable;
-import org.joda.time.Interval;
import org.junit.runner.Description;

/**
@@ -45,7 +44,7 @@
private final ListMultimap<Description, Throwable> dynamicTestToFailures
=
Multimaps.synchronizedListMultimap(LinkedListMultimap.<Description,
Throwable>create());

- @Nullable private volatile Interval runTimeInterval = null;
+ @Nullable private volatile TestInterval runTimeInterval = null;
private volatile State state = State.INITIAL;

TestCaseNode(Description description, TestSuiteNode parent) {
@@ -139,14 +138,14 @@
if (fromState == state && toState != state) {
state = toState;
runTimeInterval = runTimeInterval == null
- ? new Interval(now, now) : runTimeInterval.withEndMillis(now);
+ ? new TestInterval(now, now) :
runTimeInterval.withEndMillis(now);
return true;
}
return false;
}

@Nullable
- public Interval getRuntime() {
+ public TestInterval getRuntime() {
return runTimeInterval;
}

@@ -194,7 +193,7 @@
.build();
}

- private TestResult buildDynamicResult(Description test, @Nullable
Interval runTime,
+ private TestResult buildDynamicResult(Description test, @Nullable
TestInterval runTime,
TestResult.Status status) {
// The dynamic test fails if the testcase itself fails or there is
// a dynamic failure specifically for the dynamic test.
diff --git
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java
new file mode 100644
index 0000000..2fface1
--- /dev/null
+++
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java
@@ -0,0 +1,57 @@
+package com.google.testing.junit.runner.model;
+
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.TimeZone;
+
+import com.google.common.annotations.VisibleForTesting;
+
+/**
+ * Implementation of an immutable time interval, representing a period of
time between two isntants.
+ *
+ * This class is thread-safe and immutable.
+ */
+final class TestInterval {
+ private final long startInstant;
+ private final long endInstant;
+
+ TestInterval(long startInstant, long endInstant) {
+ if (startInstant > endInstant) {
+ throw new IllegalArgumentException("Start must be before end");
+ }
+ this.startInstant = startInstant;
+ this.endInstant = endInstant;
+ }
+
+ long getStartMillis() {
+ return startInstant;
+ }
+
+ long getEndMillis() {
+ return endInstant;
+ }
+
+ long toDurationMillis() {
+ return endInstant - startInstant;
+ }
+
+ TestInterval withEndMillis(long millis) {
+ return new TestInterval(startInstant, millis);
+ }
+
+ String startInstantToString() {
+ // Format as ISO8601 string
+ return startInstantToString(TimeZone.getDefault());
+ }
+
+ /**
+ * Exposed for testing because java Date does not allow setting of
timezones.
+ */
+ @VisibleForTesting
+ String startInstantToString(TimeZone tz) {
+ DateFormat format = new
SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSXXX");
+ format.setTimeZone(tz);
+ return format.format(new Date(startInstant));
+ }
+}
diff --git
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java
index 852f63a..06a25e2 100644
---
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java
+++
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java
@@ -17,7 +17,6 @@
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
-import org.joda.time.Interval;

/**
* Result of executing a test suite or test case.
@@ -83,7 +82,7 @@
private final Map<String, String> properties;
private final List<Throwable> failures;
@Nullable
- private final Interval runTime;
+ private final TestInterval runtime;
private final Status status;
private final int numTests, numFailures;
private final List<TestResult> childResults;
@@ -93,7 +92,7 @@
className = checkNotNull(builder.className, "className not set");
properties = checkNotNull(builder.properties, "properties not set");
failures = checkNotNull(builder.failures, "failures not set");
- runTime = builder.runTime;
+ runtime = builder.runTime;
status = checkNotNull(builder.status, "status not set");
numTests = checkNotNull(builder.numTests, "numTests not set");
numFailures = checkNotNull(builder.numFailures, "numFailures not set");
@@ -117,8 +116,8 @@
}

@Nullable
- Interval getRunTimeInterval() {
- return runTime;
+ TestInterval getRunTimeInterval() {
+ return runtime;
}

Status getStatus() {
@@ -154,7 +153,7 @@
private Map<String, String> properties = null;
private List<Throwable> failures = null;
@Nullable
- private Interval runTime = null;
+ private TestInterval runTime = null;
private Status status = null;
private Integer numTests = null;
private Integer numFailures = null;
@@ -182,7 +181,7 @@
return this;
}

- Builder runTimeInterval(@Nullable Interval runTime) {
+ Builder runTimeInterval(@Nullable TestInterval runTime) {
if (this.runTime != null) {
throw new IllegalStateException("runTime already set");
}
diff --git
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestSuiteNode.java
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestSuiteNode.java
index 437ccb1..641ad1e 100644
---
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestSuiteNode.java
+++
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestSuiteNode.java
@@ -19,7 +19,6 @@
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
-import org.joda.time.Interval;
import org.junit.runner.Description;

/**
@@ -88,7 +87,7 @@

@Override
protected TestResult buildResult() {
- Interval runTime = null;
+ TestInterval runTime = null;
int numTests = 0, numFailures = 0;
LinkedList<TestResult> childResults = new LinkedList<>();

@@ -98,10 +97,10 @@
numTests += childResult.getNumTests();
numFailures += childResult.getNumFailures();

- Interval childRunTime = childResult.getRunTimeInterval();
+ TestInterval childRunTime = childResult.getRunTimeInterval();
if (childRunTime != null) {
runTime = runTime == null ? childRunTime
- : new Interval(Math.min(runTime.getStartMillis(),
childRunTime.getStartMillis()),
+ : new TestInterval(Math.min(runTime.getStartMillis(),
childRunTime.getStartMillis()),
Math.max(runTime.getEndMillis(),
childRunTime.getEndMillis()));
}
}
diff --git a/src/java_tools/junitrunner/javatests/BUILD
b/src/java_tools/junitrunner/javatests/BUILD
new file mode 100644
index 0000000..8a45f73
--- /dev/null
+++ b/src/java_tools/junitrunner/javatests/BUILD
@@ -0,0 +1,11 @@
+java_test(
+ name = "runnerModelTests",
+ srcs = glob(["com/google/testing/junit/runner/model/*.java"]),
+ tags = ["runner"],
+ test_class = "com.google.testing.junit.runner.model.AllTests",
+ deps = [
+ "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/model",
+ "//src/test/java/com/google/devtools/build/lib:testutil",
+ "//third_party:junit4",
+ ],
+)
diff --git
a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/model/AllTests.java
b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/model/AllTests.java
new file mode 100644
index 0000000..520dd8d
--- /dev/null
+++
b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/model/AllTests.java
@@ -0,0 +1,8 @@
+package com.google.testing.junit.runner.model;
+
+import com.google.devtools.build.lib.testutil.ClasspathSuite;
+import org.junit.runner.RunWith;
+
+@RunWith(ClasspathSuite.class)
+public class AllTests {
+}
diff --git
a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/model/TestIntervalTest.java
b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/model/TestIntervalTest.java
new file mode 100644
index 0000000..390b4a5
--- /dev/null
+++
b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/model/TestIntervalTest.java
@@ -0,0 +1,48 @@
+package com.google.testing.junit.runner.model;
+
+import java.util.Date;
+import java.util.TimeZone;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+import static org.junit.Assert.assertEquals;
+
+@RunWith(JUnit4.class)
+public class TestIntervalTest {
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ @Test
+ public void testCreation() {
+ TestInterval interval = new TestInterval(123456, 234567);
+ assertEquals(123456, interval.getStartMillis());
+ assertEquals(234567, interval.getEndMillis());
+ }
+
+ @Test
+ public void testCreationFailure() {
+ thrown.expect(IllegalArgumentException.class);
+ thrown.expectMessage("Start must be before end");
+ new TestInterval(35, 23);
+ }
+
+ @Test
+ public void testToDuration() {
+ assertEquals(100, new TestInterval(50, 150).toDurationMillis());
+ assertEquals(0, new TestInterval(100, 100).toDurationMillis());
+ }
+
+ @Test
+ public void testDateFormat() {
+ Date date = new Date(1471709734000L);
+ TestInterval interval = new TestInterval(date.getTime(),
date.getTime() + 100);
+ assertEquals("2016-08-20T12:15:34.000-04:00",
interval.startInstantToString(
+ TimeZone.getTimeZone("America/New_York")));
+ assertEquals("2016-08-20T16:15:34.000Z", interval.startInstantToString(
+ TimeZone.getTimeZone("GMT")));
+ }
+}

--
To view, visit https://bazel-review.googlesource.com/5550
To unsubscribe, visit https://bazel-review.googlesource.com/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Gerrit-PatchSet: 1
Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Owner: Yunchi Luo <yun...@compass.com>

[John Cater (Gerrit)]

John Cater has posted comments on this change. (

https://bazel-review.googlesource.com/5550 )

Change subject: Remove joda-time dependency from the java test runner
......................................................................

Patch Set 1:

(3 comments)

A few comments, overall this looks good. Thanks!

https://bazel-review.googlesource.com/#/c/5550/1/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java
File
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java:

PS1, Line 20: >
Are they allowed to be equal, if a test ran for 0 milliseconds?


PS1, Line 51: VisibleForTesting
We're removing Guava from the JUnit runner as well. Just comment this
as "// VisibleForTesting" and remove the annotation (and import). See
https://github.com/bazelbuild/bazel/blob/936c2c2c815b64525bc6d3c6ac8f049655589370/src/java_tools/junitrunner/java/com/google/testing/junit/runner/sharding/RoundRobinShardingFilter.java#L39
for an example.


https://bazel-review.googlesource.com/#/c/5550/1/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java
File
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java:

PS1, Line 85: runtime
Leave this as runTime.

--
To view, visit https://bazel-review.googlesource.com/5550
To unsubscribe, visit https://bazel-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Gerrit-PatchSet: 1
Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Owner: Yunchi Luo <yun...@compass.com>

Gerrit-Reviewer: John Cater <jca...@google.com>
Gerrit-HasComments: Yes

[Yunchi Luo (Gerrit)]

Hello John Cater,

I'd like you to reexamine a change. Please visit

https://bazel-review.googlesource.com/5550

to look at the new patch set (#2).

11 files changed, 146 insertions(+), 27 deletions(-)

--
To view, visit https://bazel-review.googlesource.com/5550
To unsubscribe, visit https://bazel-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Gerrit-PatchSet: 2

Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Owner: Yunchi Luo <yun...@compass.com>

Gerrit-Reviewer: John Cater <jca...@google.com>

[Yunchi Luo (Gerrit)]

Yunchi Luo has posted comments on this change. (

https://bazel-review.googlesource.com/5550 )

Change subject: Remove joda-time dependency from the java test runner
......................................................................

Patch Set 1:

(3 comments)

https://bazel-review.googlesource.com/#/c/5550/1/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java
File
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java:

PS1, Line 20: >
> Are they allowed to be equal, if a test ran for 0 milliseconds?

Done

PS1, Line 51: VisibleForTesting
> We're removing Guava from the JUnit runner as well. Just comment this
> as "/

Done

https://bazel-review.googlesource.com/#/c/5550/1/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java
File
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java:

PS1, Line 85: runtime
> Leave this as runTime.

Huh... I have no recollection of making that change...

--
To view, visit https://bazel-review.googlesource.com/5550
To unsubscribe, visit https://bazel-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Gerrit-PatchSet: 1
Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Owner: Yunchi Luo <yun...@compass.com>

Gerrit-Reviewer: John Cater <jca...@google.com>
Gerrit-Reviewer: Yunchi Luo <yun...@compass.com>
Gerrit-HasComments: Yes

[John Cater (Gerrit)]

John Cater has posted comments on this change. (

https://bazel-review.googlesource.com/5550 )

Change subject: Remove joda-time dependency from the java test runner
......................................................................

Patch Set 2: Verified+1 Code-Review+2

Once the tests run I will merge this change. Thanks!

--
To view, visit https://bazel-review.googlesource.com/5550
To unsubscribe, visit https://bazel-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Gerrit-PatchSet: 2

Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Owner: Yunchi Luo <yun...@compass.com>

Gerrit-Reviewer: John Cater <jca...@google.com>
Gerrit-Reviewer: Yunchi Luo <yun...@compass.com>

Gerrit-HasComments: No

[Yunchi Luo (Gerrit)]

Hello John Cater, Bazel CI,

I'd like you to reexamine a change. Please visit

https://bazel-review.googlesource.com/5550

to look at the new patch set (#3).

11 files changed, 146 insertions(+), 27 deletions(-)

--
To view, visit https://bazel-review.googlesource.com/5550
To unsubscribe, visit https://bazel-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Gerrit-PatchSet: 3

Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Owner: Yunchi Luo <yun...@compass.com>

Gerrit-Reviewer: Bazel CI <ci.b...@gmail.com>

[Yunchi Luo (Gerrit)]

Yunchi Luo has posted comments on this change. (

https://bazel-review.googlesource.com/5550 )

Change subject: Remove joda-time dependency from the java test runner
......................................................................

Patch Set 1:

(1 comment)

https://bazel-review.googlesource.com/#/c/5550/1/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java
File
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java:

PS1, Line 20: >

> Done
Wait, I think the previous implementation is actually correct. They should
be allowed to be equal. Reverting.
https://github.com/bazelbuild/bazel/blob/e0029d87045712e8cbacdec2fd391c0969453fbb/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestCaseNode.java#L142

--
To view, visit https://bazel-review.googlesource.com/5550
To unsubscribe, visit https://bazel-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Gerrit-PatchSet: 1
Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Owner: Yunchi Luo <yun...@compass.com>

Gerrit-Reviewer: Bazel CI <ci.b...@gmail.com>
Gerrit-Reviewer: John Cater <jca...@google.com>
Gerrit-Reviewer: Yunchi Luo <yun...@compass.com>

Gerrit-HasComments: Yes

[John Cater (Gerrit)]

John Cater has posted comments on this change. (

https://bazel-review.googlesource.com/5550 )

Change subject: Remove joda-time dependency from the java test runner
......................................................................

Patch Set 3: Code-Review+2

That makes sense.

--
To view, visit https://bazel-review.googlesource.com/5550
To unsubscribe, visit https://bazel-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Gerrit-PatchSet: 3

Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Owner: Yunchi Luo <yun...@compass.com>

Gerrit-Reviewer: Bazel CI <ci.b...@gmail.com>
Gerrit-Reviewer: John Cater <jca...@google.com>
Gerrit-Reviewer: Yunchi Luo <yun...@compass.com>

Gerrit-HasComments: No

[Yunchi Luo (Gerrit)]

Yunchi Luo has posted comments on this change. (

https://bazel-review.googlesource.com/5550 )

Change subject: Remove joda-time dependency from the java test runner
......................................................................

Patch Set 3:

How do I queue another verification for patchset 3?

--
To view, visit https://bazel-review.googlesource.com/5550
To unsubscribe, visit https://bazel-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Gerrit-PatchSet: 3

Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Owner: Yunchi Luo <yun...@compass.com>

[John Cater (Gerrit)]

Hello Yunchi Luo, Bazel CI,

I'd like you to reexamine a change. Please visit

https://bazel-review.googlesource.com/5550

to look at the new patch set (#4).

Change subject: Remove joda-time dependency from the java test runner
......................................................................

Remove joda-time dependency from the java test runner

--
Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Reviewed-on: https://bazel-review.googlesource.com/#/c/5550/3
MOS_MIGRATED_REVID=131092172

---
M src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/AntXmlResultWriter.java
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/BUILD
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestCaseNode.java
A
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestSuiteNode.java

8 files changed, 94 insertions(+), 36 deletions(-)

--
To view, visit https://bazel-review.googlesource.com/5550
To unsubscribe, visit https://bazel-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Gerrit-PatchSet: 4

Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Owner: Yunchi Luo <yun...@compass.com>

[John Cater (Gerrit)]

John Cater has submitted this change and it was merged. (

https://bazel-review.googlesource.com/5550 )

Change subject: Remove joda-time dependency from the java test runner
......................................................................


Remove joda-time dependency from the java test runner

--
Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Reviewed-on: https://bazel-review.googlesource.com/#/c/5550/3
MOS_MIGRATED_REVID=131092172

---
M src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/AntXmlResultWriter.java
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/BUILD
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestCaseNode.java
A
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java
M
src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestSuiteNode.java

8 files changed, 94 insertions(+), 36 deletions(-)

diff --git
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
index baa4ee8..327e311 100644
--- a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
+++ b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
@@ -20,7 +20,6 @@
"//src/java_tools/junitrunner/java/com/google/testing/junit/runner/model",
"//third_party:dagger",
"//third_party:guava",
- "//third_party:joda_time",
"//third_party:junit4",
],
)
diff --git
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java

index c9c40ee..e0b94da 100644
---
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java
+++
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java
@@ -24,18 +24,14 @@
import com.google.testing.junit.runner.junit4.JUnit4RunnerModule;
import com.google.testing.junit.runner.model.AntXmlResultWriter;
import com.google.testing.junit.runner.model.XmlResultWriter;
-
import dagger.Component;
import dagger.Module;
import dagger.Provides;
-

-import org.joda.time.DateTime;
-import org.joda.time.format.DateTimeFormat;
-import org.joda.time.format.DateTimeFormatter;
-
import java.io.PrintStream;
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.util.Date;
import java.util.concurrent.TimeUnit;

-
import javax.inject.Singleton;

/**
@@ -97,9 +93,9 @@

index bfa57ae..74ab90b 100644

---
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestCaseNode.java
+++
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestCaseNode.java
@@ -31,7 +31,6 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedQueue;
import javax.annotation.Nullable;
-import org.joda.time.Interval;
import org.junit.runner.Description;

/**
@@ -45,7 +44,7 @@
private final ListMultimap<Description, Throwable> dynamicTestToFailures
=
Multimaps.synchronizedListMultimap(LinkedListMultimap.<Description,
Throwable>create());

- @Nullable private volatile Interval runTimeInterval = null;
+ @Nullable private volatile TestInterval runTimeInterval = null;
private volatile State state = State.INITIAL;

TestCaseNode(Description description, TestSuiteNode parent) {

@@ -138,15 +137,15 @@

if (fromState == state && toState != state) {
state = toState;

- runTimeInterval = runTimeInterval == null

- ? new Interval(now, now) : runTimeInterval.withEndMillis(now);

+ runTimeInterval =
+ runTimeInterval == null ? new TestInterval(now, now) :

runTimeInterval.withEndMillis(now);
return true;
}
return false;
}

@Nullable
- public Interval getRuntime() {
+ public TestInterval getRuntime() {
return runTimeInterval;
}

@@ -194,8 +193,8 @@

.build();
}

- private TestResult buildDynamicResult(Description test, @Nullable
Interval runTime,

- TestResult.Status status) {
+ private TestResult buildDynamicResult(
+ Description test, @Nullable TestInterval runTime, TestResult.Status

status) {
// The dynamic test fails if the testcase itself fails or there is
// a dynamic failure specifically for the dynamic test.

List<Throwable> dynamicFailures = dynamicTestToFailures.get(test);

diff --git
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java
new file mode 100644

index 0000000..e007a43
--- /dev/null
+++
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestInterval.java
@@ -0,0 +1,67 @@
+// Copyright 2016 The Bazel Authors. All Rights Reserved.
+//
+// Licensed 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 com.google.testing.junit.runner.model;
+
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.TimeZone;
+

+/**
+ * Implementation of an immutable time interval, representing a period of

time between two instants.
+ *
+ * <p>This class is thread-safe and immutable.

+ /** Exposed for testing because java Date does not allow setting of
timezones. */
+ // VisibleForTesting

+ String startInstantToString(TimeZone tz) {
+ DateFormat format = new
SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSXXX");
+ format.setTimeZone(tz);
+ return format.format(new Date(startInstant));
+ }
+}
diff --git
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java

index 852f63a..047b769 100644

---
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java
+++
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestResult.java
@@ -17,7 +17,6 @@
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
-import org.joda.time.Interval;

/**
* Result of executing a test suite or test case.

@@ -82,8 +81,7 @@
private final String name, className;

private final Map<String, String> properties;
private final List<Throwable> failures;

- @Nullable

- private final Interval runTime;

+ @Nullable private final TestInterval runTime;

private final Status status;
private final int numTests, numFailures;
private final List<TestResult> childResults;

@@ -117,7 +115,7 @@
}

@Nullable
- Interval getRunTimeInterval() {
+ TestInterval getRunTimeInterval() {
return runTime;
}

@@ -153,8 +151,7 @@
private String className = null;

private Map<String, String> properties = null;
private List<Throwable> failures = null;

- @Nullable

- private Interval runTime = null;

+ @Nullable private TestInterval runTime = null;

private Status status = null;
private Integer numTests = null;
private Integer numFailures = null;

@@ -182,7 +179,7 @@

return this;
}

- Builder runTimeInterval(@Nullable Interval runTime) {
+ Builder runTimeInterval(@Nullable TestInterval runTime) {
if (this.runTime != null) {
throw new IllegalStateException("runTime already set");
}
diff --git
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestSuiteNode.java
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestSuiteNode.java

index 437ccb1..2ebf6ae 100644

---
a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestSuiteNode.java
+++
b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/model/TestSuiteNode.java
@@ -19,7 +19,6 @@
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
-import org.joda.time.Interval;
import org.junit.runner.Description;

/**
@@ -88,7 +87,7 @@

@Override
protected TestResult buildResult() {
- Interval runTime = null;
+ TestInterval runTime = null;
int numTests = 0, numFailures = 0;
LinkedList<TestResult> childResults = new LinkedList<>();

@@ -98,11 +97,14 @@

numTests += childResult.getNumTests();
numFailures += childResult.getNumFailures();

- Interval childRunTime = childResult.getRunTimeInterval();
+ TestInterval childRunTime = childResult.getRunTimeInterval();
if (childRunTime != null) {

- runTime = runTime == null ? childRunTime
- : new Interval(Math.min(runTime.getStartMillis(),
childRunTime.getStartMillis()),
- Math.max(runTime.getEndMillis(),
childRunTime.getEndMillis()));
+ runTime =
+ runTime == null
+ ? childRunTime
+ : new TestInterval(
+ Math.min(runTime.getStartMillis(),
childRunTime.getStartMillis()),
+ Math.max(runTime.getEndMillis(),
childRunTime.getEndMillis()));

}
}


--
To view, visit https://bazel-review.googlesource.com/5550
To unsubscribe, visit https://bazel-review.googlesource.com/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Gerrit-PatchSet: 4

Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Owner: Yunchi Luo <yun...@compass.com>

[John Cater (Gerrit)]

John Cater has posted comments on this change. (

https://bazel-review.googlesource.com/5550 )

Change subject: Remove joda-time dependency from the java test runner
......................................................................

Patch Set 4:

> Patch Set 3:

> How do I queue another verification for patchset 3?

Only bazel admins can queue a verification run. I ran tests internally and
the patch is now merged. Thanks for contributing!

--
To view, visit https://bazel-review.googlesource.com/5550
To unsubscribe, visit https://bazel-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89804f0993e0248e23ee61c527117a9592a828da
Gerrit-PatchSet: 4

Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Owner: Yunchi Luo <yun...@compass.com>

Gerrit-Reviewer: Bazel CI <ci.b...@gmail.com>
Gerrit-Reviewer: John Cater <jca...@google.com>
Gerrit-Reviewer: Yunchi Luo <yun...@compass.com>

Gerrit-HasComments: No