Skip to content

Commit

Permalink
Change TestFactoryTestDescriptor implementation
Browse files Browse the repository at this point in the history
- Override isTest(): It is not a test but a test container
- Override hasTests(): It does not have children known at discovery
  time, but should not get pruned by launcher
- In order for PostDiscoveryFilters to be applied Root#isExcluded was
  changed to check static TestDescriptors without children instead of
  only TestDescriptors that return true for isTest()

Fixes #280.
  • Loading branch information
marcphilipp committed Jun 4, 2016
1 parent c08fc19 commit 55308d6
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ TestDescriptor getTestDescriptorFor(TestEngine testEngine) {

void applyPostDiscoveryFilters(TestDiscoveryRequest discoveryRequest) {
Filter<TestDescriptor> postDiscoveryFilter = composeFilters(discoveryRequest.getPostDiscoveryFilters());
TestDescriptor.Visitor removeExcludedTests = descriptor -> {
if (isExcludedTest(descriptor, postDiscoveryFilter)) {
TestDescriptor.Visitor removeExcludedTestDescriptors = descriptor -> {
if (isExcluded(descriptor, postDiscoveryFilter)) {
descriptor.removeFromHierarchy();
}
};
acceptInAllTestEngines(removeExcludedTests);
acceptInAllTestEngines(removeExcludedTestDescriptors);
}

/**
Expand All @@ -78,8 +78,8 @@ void prune() {
pruneEmptyTestEngines();
}

private boolean isExcludedTest(TestDescriptor descriptor, Filter<TestDescriptor> postDiscoveryFilter) {
return descriptor.isTest() && postDiscoveryFilter.apply(descriptor).excluded();
private boolean isExcluded(TestDescriptor descriptor, Filter<TestDescriptor> postDiscoveryFilter) {
return descriptor.getChildren().isEmpty() && postDiscoveryFilter.apply(descriptor).excluded();
}

private void acceptInAllTestEngines(TestDescriptor.Visitor visitor) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,11 @@ void dynamicTestsAreExecutedFromCollection() {

ExecutionEventRecorder eventRecorder = executeTests(request);

// @TestFactory methods are counted as both container and test

This comment has been minimized.

Copy link
@sbrannen

sbrannen Jun 4, 2016

Member

This is much more logical now! 👍

assertAll( //
() -> assertEquals(3, eventRecorder.getContainerStartedCount(), "# container started"),
() -> assertEquals(2, eventRecorder.getDynamicTestRegisteredCount(), "# dynamic registered"),
() -> assertEquals(3, eventRecorder.getTestStartedCount(), "# tests started"),
() -> assertEquals(2, eventRecorder.getTestSuccessfulCount(), "# tests succeeded"),
() -> assertEquals(2, eventRecorder.getTestStartedCount(), "# tests started"),
() -> assertEquals(1, eventRecorder.getTestSuccessfulCount(), "# tests succeeded"),
() -> assertEquals(1, eventRecorder.getTestFailedCount(), "# tests failed"),
() -> assertEquals(3, eventRecorder.getContainerFinishedCount(), "# container finished"));
}
Expand All @@ -110,12 +109,11 @@ void dynamicTestsAreExecutedFromIterator() {

ExecutionEventRecorder eventRecorder = executeTests(request);

// @TestFactory methods are counted as both container and test
assertAll( //
() -> assertEquals(3, eventRecorder.getContainerStartedCount(), "# container started"),
() -> assertEquals(2, eventRecorder.getDynamicTestRegisteredCount(), "# dynamic registered"),
() -> assertEquals(3, eventRecorder.getTestStartedCount(), "# tests started"),
() -> assertEquals(2, eventRecorder.getTestSuccessfulCount(), "# tests succeeded"),
() -> assertEquals(2, eventRecorder.getTestStartedCount(), "# tests started"),
() -> assertEquals(1, eventRecorder.getTestSuccessfulCount(), "# tests succeeded"),
() -> assertEquals(1, eventRecorder.getTestFailedCount(), "# tests failed"),
() -> assertEquals(3, eventRecorder.getContainerFinishedCount(), "# container finished"));
}
Expand All @@ -131,8 +129,8 @@ void dynamicTestsAreExecutedFromIterable() {
assertAll( //
() -> assertEquals(3, eventRecorder.getContainerStartedCount(), "# container started"),
() -> assertEquals(2, eventRecorder.getDynamicTestRegisteredCount(), "# dynamic registered"),
() -> assertEquals(3, eventRecorder.getTestStartedCount(), "# tests started"),
() -> assertEquals(2, eventRecorder.getTestSuccessfulCount(), "# tests succeeded"),
() -> assertEquals(2, eventRecorder.getTestStartedCount(), "# tests started"),
() -> assertEquals(1, eventRecorder.getTestSuccessfulCount(), "# tests succeeded"),
() -> assertEquals(1, eventRecorder.getTestFailedCount(), "# tests failed"),
() -> assertEquals(3, eventRecorder.getContainerFinishedCount(), "# container finished"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,21 @@ public TestFactoryTestDescriptor(UniqueId uniqueId, Class<?> testClass, Method t
super(uniqueId, testClass, testMethod);
}

@Override
public boolean isTest() {
return false;
}

@Override
public boolean isContainer() {
return true;
}

@Override
public boolean hasTests() {
return true;
}

@Override
public boolean isLeaf() {
return true;
Expand Down

0 comments on commit 55308d6

Please sign in to comment.