EngineTestKit started Executions same as finished

[tonyhallett]

Given that there is only Execution.skipped and Execution.finished

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

Line 253 in ee3a100

private static List<Execution> createExecutions(List<Event> events) {

	private static List<Execution> createExecutions(List<Event> events) {
		List<Execution> executions = new ArrayList<>();
		Map<TestDescriptor, Instant> executionStarts = new HashMap<>();

		for (Event event : events) {
			switch (event.getType()) {
				case STARTED -> executionStarts.put(event.getTestDescriptor(), event.getTimestamp());
				case SKIPPED -> {
					// 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);
				}
				case FINISHED -> {
					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);
					}
				}
				default -> {
					// Ignore other events
				}
			}
		}

		return executions;
	}

and skipped and finished have associated TerminationInfo

https://github.com/junit-team/junit-framework/blob/main/junit-platform-testkit/src/main/java/org/junit/platform/testkit/engine/Execution.java

	public static Execution finished(TestDescriptor testDescriptor, Instant startInstant, Instant endInstant,
			TestExecutionResult executionResult) {

		return new Execution(testDescriptor, startInstant, endInstant, TerminationInfo.executed(executionResult));
	}

	public static Execution skipped(TestDescriptor testDescriptor, Instant startInstant, Instant endInstant,
			String skipReason) {

		return new Execution(testDescriptor, startInstant, endInstant, TerminationInfo.skipped(skipReason));
	}

https://github.com/junit-team/junit-framework/blob/main/junit-platform-testkit/src/main/java/org/junit/platform/testkit/engine/TerminationInfo.java

	public static TerminationInfo skipped(@Nullable String reason) {
		return new TerminationInfo(true, reason, null);
	}

	public static TerminationInfo executed(TestExecutionResult testExecutionResult) {
		Preconditions.notNull(testExecutionResult, "TestExecutionResult must not be null");
		return new TerminationInfo(false, null, testExecutionResult);
	}
	private TerminationInfo(boolean skipped, @Nullable String skipReason,
			@Nullable TestExecutionResult testExecutionResult) {
		boolean executed = (testExecutionResult != null);
		Preconditions.condition((skipped ^ executed),
			"TerminationInfo must represent either a skipped execution or a TestExecutionResult but not both");

		this.skipped = skipped;
		this.skipReason = skipReason;
		this.testExecutionResult = testExecutionResult;
	}

and started are those that have not been skipped

	public Executions started() {
		return new Executions(executionsByTerminationInfo(TerminationInfo::notSkipped), this.category + " Started");
	}
	private Stream<Execution> executionsByTerminationInfo(Predicate<TerminationInfo> predicate) {
		return filter(execution -> predicate.test(execution.getTerminationInfo()));
	}

then this will be all executions that have finished, given that they were not skipped.

So there is no way to determine if the engine has invoked executionStarted but has failed to follow that up with executionSkipped or executionFinished.

The associated test

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

Line 44 in ee3a100

assertThat(testEvents.executions().started().count()).isEqualTo(3);

which uses the well behaved junit-jupiter, asserting started ( "finished").

Then started is all those executions that were created

[mpkorstanje]

Thanks for the report. Unfortunately it looks like I'm reading the investigation and conclusion of bug, but I have to read the between the lines to infer what the actual bug is.

For clarity and expediency could you make it explicit:

1. what the (faulty?) behaviour is of the engine you are testing.
2. the exact scenario that is being used to test the engine.
3. what the expected and actual outcomes of that test are.

[mpkorstanje]

So there is no way to determine if the engine has invoked executionStarted but has failed to follow that up with executionSkipped or executionFinished.

Specifically, I'm quite puzzled by the existence of a scenario where a test should positively assert that executionStarted is not followed by an executionFinished. That would suggest an intentionally faulty engine implementation. At first glance this is not a usecase that needs support.

Edit: Doesn't need support in Executions, but rather could be tested by the slightly lower level Events.

[tonyhallett]

1. engine has invoked executionStarted and followed that up with executionSkipped or executionFinished.

		class EngineTestKitSkippedMethodDemo {
		
			@Test
			void verifyJupiterMethodWasSkipped() {
				String methodName = "skippedTest";
		
				var numStartedExecutions = EngineTestKit
					.engine("good-engine")
					.selectors(selectMethod(ExampleTestCase.class, methodName))
					.execute()
                                        .testEvents().executions().started().count();
                               
                               assertEquals(0,numStartedExecutions)

                               

3. If my engine failed to behave properly then there should be start executions and if it did then there should be none.

Specifically, I'm quite puzzled by the existence of a scenario where a test should positively assert that executionStarted is not followed by an executionFinished.

It's not, it's asserting the opposite. That it is not faulty.

I am puzzled why started() and finished() provide the same value ?

[mpkorstanje]

1. engine has invoked executionStarted and followed that up with executionSkipped or executionFinished.

One of these behaviors is faulty.

An executionStarted should always be followed by a executionFinished and never a executionSkipped.

junit-framework/junit-platform-engine/src/main/java/org/junit/platform/engine/EngineExecutionListener.java

Lines 63 to 65 in f710172

	* <p>A skipped test or subtree of tests must not be reported as
	* {@linkplain #executionStarted started} or
	* {@linkplain #executionFinished finished}.

JUnit makes a distinction between tests that were never started (skipped) and tests that were started, and then skipped during execution (aborted). If a test is started and then "skipped" during execution, executionFinished should be used with TestExecutionResult.Status.ABORTED.

[mpkorstanje]

I am puzzled why started() and finished() provide the same value ?

I can't tell from your example what events your test engine emits. Could you share the events logged by using:

var numStartedExecutions = EngineTestKit
	.engine("good-engine")
	.selectors(selectMethod(ExampleTestCase.class, methodName))
	.execute()
	.testEvents().debug()
	.executions().started().count();

[tonyhallett]

One of these behaviors is faulty.

Correct. I read the code and not the comment before that.

Regardless, started still returns those that have finished, so is an alias. I think this should be clear in the doc comments.

Execution encapsulates metadata for the execution of a single TestDescriptor

Without reading the members you would not know that Execution was only for skipped or finished. That there are no start executions for executions that have started and not finished.

[tonyhallett]

I can't tell from your example what events your test engine emits.

I don't have a test engine.

[tonyhallett]

I'll use EventStatistics for this purpose. Although Execution with open starts would allow determining the TestDescriptor that did not finish.