Skip to content

Commit 4fd209f

Browse files
brasmussonbrasmusson
committed
[Core] Use the test case duration in the JUnit Formatter.
Let the JUnit Formatter use the duration of the test case result instead of summing up the durations of the test step results. Also refactor the TimeService to have the methods startTime(), currentTime(), and finishTime(). These methods actually do the same thing but having them all enables/simplifies the mocking of time in tests.
1 parent 55b1f10 commit 4fd209f

File tree

17 files changed

+94
-53
lines changed

17 files changed

+94
-53
lines changed

‎android/src/main/java/cucumber/runtime/android/CucumberExecutor.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public void execute() {
111111
runtime.getRunner().runPickle(pickleEvent);
112112
}
113113

114-
runtime.getEventBus().send(new TestRunFinished(runtime.getEventBus().getTime()));
114+
runtime.getEventBus().send(new TestRunFinished(runtime.getEventBus().finishTime()));
115115
}
116116

117117
/**

‎core/src/main/java/cucumber/api/TestCase.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public TestCase(List<TestStep> testSteps, PickleEvent pickleEvent) {
2121

2222
public void run(EventBus bus) {
2323
boolean skipNextStep = false;
24-
Long startTime = bus.getTime();
24+
Long startTime = bus.startTime();
2525
bus.send(new TestCaseStarted(startTime, this));
2626
ScenarioImpl scenarioResult = new ScenarioImpl(bus, pickleEvent);
2727
for (TestStep step : testSteps) {
@@ -31,7 +31,7 @@ public void run(EventBus bus) {
3131
}
3232
scenarioResult.add(stepResult);
3333
}
34-
Long stopTime = bus.getTime();
34+
Long stopTime = bus.finishTime();
3535
bus.send(new TestCaseFinished(stopTime, this, new Result(scenarioResult.getStatus(), stopTime - startTime, scenarioResult.getError())));
3636
}
3737

‎core/src/main/java/cucumber/api/TestStep.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public List<cucumber.runtime.Argument> getDefinitionArgument() {
5252
public abstract HookType getHookType();
5353

5454
public Result run(EventBus bus, String language, Scenario scenario, boolean skipSteps) {
55-
Long startTime = bus.getTime();
55+
Long startTime = bus.startTime();
5656
bus.send(new TestStepStarted(startTime, this));
5757
Result.Type status;
5858
Throwable error = null;
@@ -62,7 +62,7 @@ public Result run(EventBus bus, String language, Scenario scenario, boolean skip
6262
error = t;
6363
status = mapThrowableToStatus(t);
6464
}
65-
Long stopTime = bus.getTime();
65+
Long stopTime = bus.finishTime();
6666
Result result = mapStatusToResult(status, error, stopTime - startTime);
6767
bus.send(new TestStepFinished(stopTime, this, result));
6868
return result;

‎core/src/main/java/cucumber/runner/EventBus.java

+10-2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,16 @@ public EventBus(TimeService stopWatch) {
1717
this.stopWatch = stopWatch;
1818
}
1919

20-
public Long getTime() {
21-
return stopWatch.time();
20+
public Long startTime() {
21+
return stopWatch.startTime();
22+
}
23+
24+
public Long finishTime() {
25+
return stopWatch.finishTime();
26+
}
27+
28+
public Long currentTime() {
29+
return stopWatch.currentTime();
2230
}
2331

2432
public void send(Event event) {

‎core/src/main/java/cucumber/runner/TimeService.java

+47-5
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,70 @@
11
package cucumber.runner;
22

33
public interface TimeService {
4-
long time();
4+
// The reason to have three method, which actually is doing the same thing,
5+
// is to enable stubbing of the time service in tests. With the distinction
6+
// between startTime() and finishTime() it is easy for the stub to make sure
7+
// the the duration between the start time and finish time of the test case,
8+
// is the sum of the durations of the test steps.
9+
long startTime();
10+
long currentTime();
11+
long finishTime();
512

613
TimeService SYSTEM = new TimeService() {
714
@Override
8-
public long time() {
15+
public long startTime() {
16+
return currentTime();
17+
}
18+
19+
@Override
20+
public long currentTime() {
921
return System.nanoTime();
1022
}
23+
24+
@Override
25+
public long finishTime() {
26+
return currentTime();
27+
}
1128
};
1229

1330
class Stub implements TimeService {
1431
private final long duration;
1532
private final ThreadLocal<Long> currentTime = new ThreadLocal<Long>();
33+
private boolean startTimeWasLastCall = false;
1634

1735
public Stub(long duration) {
1836
this.duration = duration;
1937
}
2038

2139
@Override
22-
public long time() {
40+
public long startTime() {
41+
startTimeWasLastCall = true;
42+
return getCurrentTime();
43+
}
44+
45+
@Override
46+
public long finishTime() {
47+
return currentTime();
48+
}
49+
50+
@Override
51+
public long currentTime() {
52+
long result = getCurrentTime();
53+
result = incrementTimeIfstartTimeWasLastCall(result);
54+
return result;
55+
}
56+
57+
private long getCurrentTime() {
2358
Long result = currentTime.get();
24-
result = result != null ? result : 0l;
25-
currentTime.set(result + duration);
59+
return result != null ? result : 0l;
60+
}
61+
62+
private long incrementTimeIfstartTimeWasLastCall(long result) {
63+
if (startTimeWasLastCall) {
64+
result += duration;
65+
currentTime.set(result);
66+
startTimeWasLastCall = false;
67+
}
2668
return result;
2769
}
2870
}

‎core/src/main/java/cucumber/runtime/Runtime.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ public void run() throws IOException {
148148
runFeature(cucumberFeature);
149149
}
150150

151-
bus.send(new TestRunFinished(bus.getTime()));
151+
bus.send(new TestRunFinished(bus.finishTime()));
152152
printSummary();
153153
}
154154

‎core/src/main/java/cucumber/runtime/ScenarioImpl.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,14 @@ public boolean isFailed() {
7575
@Override
7676
public void embed(byte[] data, String mimeType) {
7777
if (bus != null) {
78-
bus.send(new EmbedEvent(bus.getTime(), data, mimeType));
78+
bus.send(new EmbedEvent(bus.currentTime(), data, mimeType));
7979
}
8080
}
8181

8282
@Override
8383
public void write(String text) {
8484
if (bus != null) {
85-
bus.send(new WriteEvent(bus.getTime(), text));
85+
bus.send(new WriteEvent(bus.currentTime(), text));
8686
}
8787
}
8888

‎core/src/main/java/cucumber/runtime/formatter/JUnitFormatter.java

+6-17
Original file line numberDiff line numberDiff line change
@@ -126,14 +126,12 @@ private void handleTestStepFinished(TestStepFinished event) {
126126
if (!event.testStep.isHook()) {
127127
testCase.steps.add(event.testStep);
128128
testCase.results.add(event.result);
129-
} else {
130-
testCase.hookResults.add(event.result);
131129
}
132130
}
133131

134132
private void handleTestCaseFinished(TestCaseFinished event) {
135133
if (testCase.steps.isEmpty()) {
136-
testCase.handleEmptyTestCase(doc, root);
134+
testCase.handleEmptyTestCase(doc, root, event.result);
137135
} else {
138136
testCase.addTestCaseElement(doc, root, event.result);
139137
}
@@ -219,7 +217,6 @@ private TestCase(cucumber.api.TestCase testCase) {
219217
static boolean treatConditionallySkippedAsFailure = false;
220218
final List<TestStep> steps = new ArrayList<TestStep>();
221219
final List<Result> results = new ArrayList<Result>();
222-
final List<Result> hookResults = new ArrayList<Result>();
223220
private final cucumber.api.TestCase testCase;
224221

225222
private Element createElement(Document doc) {
@@ -247,7 +244,7 @@ private boolean includesBlank(String testCaseName) {
247244
}
248245

249246
public void addTestCaseElement(Document doc, Element tc, Result result) {
250-
tc.setAttribute("time", calculateTotalDurationString());
247+
tc.setAttribute("time", calculateTotalDurationString(result));
251248

252249
StringBuilder sb = new StringBuilder();
253250
addStepAndResultListing(sb);
@@ -272,25 +269,17 @@ public void addTestCaseElement(Document doc, Element tc, Result result) {
272269
tc.appendChild(child);
273270
}
274271

275-
public void handleEmptyTestCase(Document doc, Element tc) {
276-
tc.setAttribute("time", calculateTotalDurationString());
272+
public void handleEmptyTestCase(Document doc, Element tc, Result result) {
273+
tc.setAttribute("time", calculateTotalDurationString(result));
277274

278275
String resultType = treatConditionallySkippedAsFailure ? "failure" : "skipped";
279276
Element child = createElementWithMessage(doc, new StringBuilder(), resultType, "The scenario has no steps");
280277

281278
tc.appendChild(child);
282279
}
283280

284-
private String calculateTotalDurationString() {
285-
long totalDurationNanos = 0;
286-
for (Result r : results) {
287-
totalDurationNanos += r.getDuration() == null ? 0 : r.getDuration();
288-
}
289-
for (Result r : hookResults) {
290-
totalDurationNanos += r.getDuration() == null ? 0 : r.getDuration();
291-
}
292-
double totalDurationSeconds = ((double) totalDurationNanos) / 1000000000;
293-
return NUMBER_FORMAT.format(totalDurationSeconds);
281+
private String calculateTotalDurationString(Result result) {
282+
return NUMBER_FORMAT.format(((double) result.getDuration()) / 1000000000);
294283
}
295284

296285
private void addStepAndResultListing(StringBuilder sb) {

‎core/src/main/java/cucumber/runtime/model/CucumberFeature.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ public String getPath() {
139139
}
140140

141141
public void sendTestSourceRead(EventBus bus) {
142-
bus.send(new TestSourceRead(bus.getTime(), path, gherkinDocument.getFeature().getLanguage(), gherkinSource));
142+
bus.send(new TestSourceRead(bus.currentTime(), path, gherkinDocument.getFeature().getLanguage(), gherkinSource));
143143
}
144144

145145
private void setLanguage(String language) {

‎core/src/test/java/cucumber/api/TestStepTest.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,8 @@ public void result_is_pending_when_step_definition_throws_pending_exception() th
9191
public void step_execution_time_is_measured() throws Throwable {
9292
Long duration = new Long(1234);
9393
TestStep step = new PickleTestStep("uri", mock(PickleStep.class), definitionMatch);
94-
when(bus.getTime()).thenReturn(0l, 1234l);
94+
when(bus.startTime()).thenReturn(0L);
95+
when(bus.finishTime()).thenReturn(1234L);
9596

9697
Result result = step.run(bus, language, scenario, false);
9798

‎core/src/test/java/cucumber/runner/TimeServiceTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@ public TimerThread(long timeoutMillis) {
3737
@Override
3838
public void run() {
3939
try {
40-
stopWatch.time();
40+
stopWatch.currentTime();
4141
Thread.sleep(timeoutMillis);
42-
stopWatch.time();
42+
stopWatch.currentTime();
4343
} catch (NullPointerException e) {
4444
exception = e;
4545
} catch (InterruptedException e) {

‎core/src/test/java/cucumber/runtime/TestHelper.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ public static void runFeaturesWithFormatter(final List<CucumberFeature> features
148148
feature.sendTestSourceRead(runtime.getEventBus());
149149
runtime.runFeature(feature);
150150
}
151-
runtime.getEventBus().send(new TestRunFinished(runtime.getEventBus().getTime()));
151+
runtime.getEventBus().send(new TestRunFinished(runtime.getEventBus().finishTime()));
152152
}
153153

154154
private static RuntimeGlue createMockedRuntimeGlueThatMatchesTheSteps(final Map<String, Result> stepsToResult, final Map<String, String> stepsToLocation,

‎core/src/test/java/cucumber/runtime/formatter/JSONFormatterTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ public void should_handle_write_from_a_hooks() throws Throwable {
516516
" ],\n" +
517517
" \"result\": {\n" +
518518
" \"status\": \"passed\",\n" +
519-
" \"duration\": 2000000\n" +
519+
" \"duration\": 1000000\n" +
520520
" }\n" +
521521
" }\n" +
522522
" ],\n" +
@@ -592,7 +592,7 @@ public void should_handle_embed_from_a_hooks() throws Throwable {
592592
" ],\n" +
593593
" \"result\": {\n" +
594594
" \"status\": \"passed\",\n" +
595-
" \"duration\": 2000000\n" +
595+
" \"duration\": 1000000\n" +
596596
" }\n" +
597597
" }\n" +
598598
" ],\n" +

‎core/src/test/java/cucumber/runtime/formatter/JUnitFormatterTest.java

+12-11
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cucumber.runtime.formatter;
22

33
import cucumber.api.Result;
4+
import cucumber.runner.TimeService;
45
import cucumber.runtime.Backend;
56
import cucumber.runtime.Runtime;
67
import cucumber.runtime.RuntimeOptions;
@@ -116,8 +117,8 @@ public void should_format_skipped_scenario() throws Throwable {
116117

117118
String stackTrace = getStackTrace(exception);
118119
String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n" +
119-
"<testsuite failures=\"0\" name=\"cucumber.runtime.formatter.JUnitFormatter\" skipped=\"1\" tests=\"1\" time=\"0.001\">\n" +
120-
" <testcase classname=\"path/test.feature\" name=\"scenario name\" time=\"0.001\">\n" +
120+
"<testsuite failures=\"0\" name=\"cucumber.runtime.formatter.JUnitFormatter\" skipped=\"1\" tests=\"1\" time=\"0.003\">\n" +
121+
" <testcase classname=\"path/test.feature\" name=\"scenario name\" time=\"0.003\">\n" +
121122
" <skipped message=\"" + stackTrace.replace("\n\t", "&#10;&#9;") + "\"><![CDATA[" +
122123
"Given first step............................................................skipped\n" +
123124
"When second step............................................................skipped\n" +
@@ -148,8 +149,8 @@ public void should_format_pending_scenario() throws Throwable {
148149
String formatterOutput = runFeatureWithJUnitFormatter(feature, stepsToResult, stepDuration);
149150

150151
String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n" +
151-
"<testsuite failures=\"0\" name=\"cucumber.runtime.formatter.JUnitFormatter\" skipped=\"1\" tests=\"1\" time=\"0.001\">\n" +
152-
" <testcase classname=\"path/test.feature\" name=\"scenario name\" time=\"0.001\">\n" +
152+
"<testsuite failures=\"0\" name=\"cucumber.runtime.formatter.JUnitFormatter\" skipped=\"1\" tests=\"1\" time=\"0.003\">\n" +
153+
" <testcase classname=\"path/test.feature\" name=\"scenario name\" time=\"0.003\">\n" +
153154
" <skipped><![CDATA[" +
154155
"Given first step............................................................pending\n" +
155156
"When second step............................................................skipped\n" +
@@ -211,8 +212,8 @@ public void should_handle_failure_in_before_hook() throws Throwable {
211212
String formatterOutput = runFeatureWithJUnitFormatter(feature, stepsToResult, hooks, stepHookDuration);
212213

213214
String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n" +
214-
"<testsuite failures=\"1\" name=\"cucumber.runtime.formatter.JUnitFormatter\" skipped=\"0\" tests=\"1\" time=\"0.001\">\n" +
215-
" <testcase classname=\"path/test.feature\" name=\"scenario name\" time=\"0.001\">\n" +
215+
"<testsuite failures=\"1\" name=\"cucumber.runtime.formatter.JUnitFormatter\" skipped=\"0\" tests=\"1\" time=\"0.004\">\n" +
216+
" <testcase classname=\"path/test.feature\" name=\"scenario name\" time=\"0.004\">\n" +
216217
" <failure message=\"the stack trace\"><![CDATA[" +
217218
"Given first step............................................................skipped\n" +
218219
"When second step............................................................skipped\n" +
@@ -245,8 +246,8 @@ public void should_handle_pending_in_before_hook() throws Throwable {
245246
String formatterOutput = runFeatureWithJUnitFormatter(feature, stepsToResult, hooks, stepHookDuration);
246247

247248
String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n" +
248-
"<testsuite failures=\"0\" name=\"cucumber.runtime.formatter.JUnitFormatter\" skipped=\"1\" tests=\"1\" time=\"0.001\">\n" +
249-
" <testcase classname=\"path/test.feature\" name=\"scenario name\" time=\"0.001\">\n" +
249+
"<testsuite failures=\"0\" name=\"cucumber.runtime.formatter.JUnitFormatter\" skipped=\"1\" tests=\"1\" time=\"0.004\">\n" +
250+
" <testcase classname=\"path/test.feature\" name=\"scenario name\" time=\"0.004\">\n" +
250251
" <skipped><![CDATA[" +
251252
"Given first step............................................................skipped\n" +
252253
"When second step............................................................skipped\n" +
@@ -277,8 +278,8 @@ public void should_handle_failure_in_before_hook_with_background() throws Throwa
277278
String formatterOutput = runFeatureWithJUnitFormatter(feature, stepsToResult, hooks, stepHookDuration);
278279

279280
String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n" +
280-
"<testsuite failures=\"1\" name=\"cucumber.runtime.formatter.JUnitFormatter\" skipped=\"0\" tests=\"1\" time=\"0.001\">\n" +
281-
" <testcase classname=\"path/test.feature\" name=\"scenario name\" time=\"0.001\">\n" +
281+
"<testsuite failures=\"1\" name=\"cucumber.runtime.formatter.JUnitFormatter\" skipped=\"0\" tests=\"1\" time=\"0.004\">\n" +
282+
" <testcase classname=\"path/test.feature\" name=\"scenario name\" time=\"0.004\">\n" +
282283
" <failure message=\"the stack trace\"><![CDATA[" +
283284
"Given first step............................................................skipped\n" +
284285
"When second step............................................................skipped\n" +
@@ -535,7 +536,7 @@ private File runFeaturesWithJunitFormatter(final List<String> featurePaths, bool
535536
RuntimeOptions runtimeOptions = new RuntimeOptions(args);
536537
Backend backend = mock(Backend.class);
537538
when(backend.getSnippet(any(PickleStep.class), anyString(), any(FunctionNameGenerator.class))).thenReturn("TEST SNIPPET");
538-
final cucumber.runtime.Runtime runtime = new Runtime(resourceLoader, classLoader, asList(backend), runtimeOptions);
539+
final cucumber.runtime.Runtime runtime = new Runtime(resourceLoader, classLoader, asList(backend), runtimeOptions, new TimeService.Stub(0L), null);
539540
runtime.run();
540541
return report;
541542
}

‎core/src/test/java/cucumber/runtime/formatter/PluginFactoryTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public void plugin_does_not_buffer_its_output() throws IOException {
9797
EventBus bus = new EventBus(new TimeService.Stub(0));
9898
plugin.setEventPublisher(bus);
9999
Result result = new Result(Result.Type.PASSED, null, null);
100-
TestStepFinished event = new TestStepFinished(bus.getTime(), mock(TestStep.class), result);
100+
TestStepFinished event = new TestStepFinished(bus.finishTime(), mock(TestStep.class), result);
101101
bus.send(event);
102102

103103
assertThat(mockSystemOut.toString(), is(not("")));

‎junit/src/main/java/cucumber/api/junit/Cucumber.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ protected void runChild(FeatureRunner child, RunNotifier notifier) {
101101
@Override
102102
public void run(RunNotifier notifier) {
103103
super.run(notifier);
104-
runtime.getEventBus().send(new TestRunFinished(runtime.getEventBus().getTime()));
104+
runtime.getEventBus().send(new TestRunFinished(runtime.getEventBus().finishTime()));
105105
runtime.printSummary();
106106
}
107107

‎testng/src/main/java/cucumber/api/testng/TestNGCucumberRunner.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public void runCucumber(CucumberFeature cucumberFeature) {
6969
}
7070

7171
public void finish() {
72-
runtime.getEventBus().send(new TestRunFinished(runtime.getEventBus().getTime()));
72+
runtime.getEventBus().send(new TestRunFinished(runtime.getEventBus().finishTime()));
7373
runtime.printSummary();
7474
}
7575

0 commit comments

Comments
 (0)