Avoid NullPointerException when creating Executions from filtered events

Prior to this commit, the logic in Executions.createExecutions() could
result in a NullPointerException if the executions were being created
from filtered events.

For example, an attempt to create executions from
`testEvents.failed().executions()` resulted in a NullPointerException
since the "started" event (needed to determine the start Instant) was
no longer present in the event list.

This commit fixes this bug by avoiding the creation of Execution
instances if the "started" event is absent.

Consequently, an invocation such as
`testEvents.failed().executions().count()` now returns `0` instead of
throwing a NullPointerException.

Issue: #1356

Author: sbrannen

junit-platform-testkit/src/main/java/org/junit/platform/testkit/engine/Executions.java

@@ -261,20 +261,32 @@ private static List<Execution> createExecutions(List<Event> events) {
 					break;
 				}
 				case SKIPPED: {
-					Instant startInstant = executionStarts.get(event.getTestDescriptor());
-					Execution skippedEvent = Execution.skipped(event.getTestDescriptor(),
-						startInstant != null ? startInstant : event.getTimestamp(), event.getTimestamp(),
+					// Based on the Javadoc for EngineExecutionListener.executionSkipped(...),
+					// a skipped descriptor must never be reported as started or finished,
+					// but just in case a TestEngine does not adhere to that contract, we
+					// make an attempt to remove any tracked "started" event.
+					executionStarts.remove(event.getTestDescriptor());
+
+					// Use the same timestamp for both the start and end Instant values.
+					Instant timestamp = event.getTimestamp();
+
+					Execution skippedEvent = Execution.skipped(event.getTestDescriptor(), timestamp, timestamp,
 						event.getRequiredPayload(String.class));
 					executions.add(skippedEvent);
-					executionStarts.remove(event.getTestDescriptor());
 					break;
 				}
 				case FINISHED: {
-					Execution finishedEvent = Execution.finished(event.getTestDescriptor(),
-						executionStarts.get(event.getTestDescriptor()), event.getTimestamp(),
-						event.getRequiredPayload(TestExecutionResult.class));
-					executions.add(finishedEvent);
-					executionStarts.remove(event.getTestDescriptor());
+					Instant startInstant = executionStarts.remove(event.getTestDescriptor());
+					Instant endInstant = event.getTimestamp();
+
+					// If "started" events have already been filtered out, it is no
+					// longer possible to create a legitimate Execution for the current
+					// descriptor. So we effectively ignore it in that case.
+					if (startInstant != null) {
+						Execution finishedEvent = Execution.finished(event.getTestDescriptor(), startInstant,
+							endInstant, event.getRequiredPayload(TestExecutionResult.class));
+						executions.add(finishedEvent);
+					}
 					break;
 				}
 				default: {

platform-tests/src/test/java/org/junit/platform/testkit/engine/ExecutionsIntegrationTests.java

@@ -0,0 +1,103 @@
+/*
+ * Copyright 2015-2018 the original author or authors.
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v2.0 which
+ * accompanies this distribution and is available at
+ *
+ * http://www.eclipse.org/legal/epl-v20.html
+ */
+
+package org.junit.platform.testkit.engine;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
+
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Integration tests for {@link Executions}.
+ *
+ * @since 1.4
+ */
+class ExecutionsIntegrationTests {
+
+	@Test
+	void executionsFromSkippedTestEvents() {
+		Events testEvents = getTestEvents();
+
+		// We expect 1 for both of the following cases, since an Execution can
+		// be created for a "skipped event even if "started" and "finished" events
+		// are filtered out.
+		assertThat(testEvents.executions().skipped().count()).isEqualTo(1);
+		assertThat(testEvents.skipped().executions().count()).isEqualTo(1);
+	}
+
+	@Test
+	void executionsFromSucceededTestEvents() {
+		Events testEvents = getTestEvents();
+
+		// We expect 1 if the executions are created BEFORE filtering out "finished" events.
+		assertThat(testEvents.executions().succeeded().count()).isEqualTo(1);
+		// We expect 0 if the executions are created AFTER filtering out "finished" events.
+		assertThat(testEvents.succeeded().executions().count()).isEqualTo(0);
+	}
+
+	@Test
+	void executionsFromAbortedTestEvents() {
+		Events testEvents = getTestEvents();
+
+		// We expect 1 if the executions are created BEFORE filtering out "started" events.
+		assertThat(testEvents.executions().aborted().count()).isEqualTo(1);
+		// We expect 0 if the executions are created AFTER filtering out "started" events.
+		assertThat(testEvents.aborted().executions().count()).isEqualTo(0);
+	}
+
+	@Test
+	void executionsFromFailedTestEvents() {
+		Events testEvents = getTestEvents();
+
+		// We expect 1 if the executions are created BEFORE filtering out "started" events.
+		assertThat(testEvents.executions().failed().count()).isEqualTo(1);
+		// We expect 0 if the executions are created AFTER filtering out "started" events.
+		assertThat(testEvents.failed().executions().count()).isEqualTo(0);
+	}
+
+	private Events getTestEvents() {
+		Events testEvents = EngineTestKit.engine("junit-jupiter")//
+				.selectors(selectClass(ExampleTestCase.class))//
+				.execute()//
+				.tests();
+
+		testEvents.assertStatistics(stats -> stats.skipped(1).started(3).succeeded(1).aborted(1).failed(1));
+
+		return testEvents;
+	}
+
+	static class ExampleTestCase {
+
+		@Test
+		@Disabled
+		void skippedTest() {
+		}
+
+		@Test
+		void succeedingTest() {
+		}
+
+		@Test
+		void abortedTest() {
+			assumeTrue(false);
+		}
+
+		@Test
+		void failingTest() {
+			fail("Boom!");
+		}
+
+	}
+
+}