Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow having YamlTestExtension include method name in test names #3253

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,23 @@ public class YamlTestExtension implements TestTemplateInvocationContextProvider,
private List<ExternalServer> servers;
@Nullable
private final String clusterFile;
private final boolean includeMethodInDescriptions;

public YamlTestExtension() {
this.clusterFile = null; // it will get it from the environment
this(null, false);
}

public YamlTestExtension(@Nullable final String clusterFile) {
/**
* Create a new extension with some configuration.
* @param clusterFile a custom cluster file to use, or {@code null} to inherit it from the environment, namely
* {@code FDB_CLUSTER_FILE}.
* @param includeMethodInDescriptions Set this to {@code true} if publishing test results to something that cannot
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just set this to true always? Does the name look bad when running in an IDE or something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, including the method name is generally redundant, and noisy.
image
vs
image

And in the gradle output, you would have:

2025-03-17T21:19:14.4449128Z 
2025-03-17T21:19:14.4449615Z 2025-03-17T21:19:14.375445846Z YamlIntegrationTests > showcasingTests(Runner) > Embedded STARTED
2025-03-17T21:19:14.5447904Z 2025-03-17T21:19:14.496453802Z YamlIntegrationTests > showcasingTests(Runner) > Embedded SUCCESS (121ms)
2025-03-17T21:19:14.5448575Z 
2025-03-17T21:19:14.5449089Z 2025-03-17T21:19:14.496870499Z YamlIntegrationTests > showcasingTests(Runner) > JDBC In-Process STARTED
2025-03-17T21:19:14.6447848Z 2025-03-17T21:19:14.626601851Z YamlIntegrationTests > showcasingTests(Runner) > JDBC In-Process SUCCESS (130ms)
2025-03-17T21:19:14.6448569Z 
2025-03-17T21:19:14.6450225Z 2025-03-17T21:19:14.627089079Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (Embedded then !current_version) STARTED
2025-03-17T21:19:14.8448278Z 2025-03-17T21:19:14.793403399Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (Embedded then !current_version) SUCCESS (166ms)
2025-03-17T21:19:14.8448824Z 
2025-03-17T21:19:14.8449315Z 2025-03-17T21:19:14.794018940Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (Embedded then !current_version) WITH {optionForceContinuation=true} STARTED
2025-03-17T21:19:15.0448535Z 2025-03-17T21:19:14.964409459Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (Embedded then !current_version) WITH {optionForceContinuation=true} SUCCESS (171ms)
2025-03-17T21:19:15.0449586Z 
2025-03-17T21:19:15.0450027Z 2025-03-17T21:19:14.964792102Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (!current_version then Embedded) STARTED
2025-03-17T21:19:15.1448164Z 2025-03-17T21:19:15.119398097Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (!current_version then Embedded) SUCCESS (156ms)
2025-03-17T21:19:15.1448728Z 
2025-03-17T21:19:15.1449230Z 2025-03-17T21:19:15.119984738Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (!current_version then Embedded) WITH {optionForceContinuation=true} STARTED
2025-03-17T21:19:15.3448437Z 2025-03-17T21:19:15.300290885Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (!current_version then Embedded) WITH {optionForceContinuation=true} SUCCESS (180ms)
2025-03-17T21:19:15.3449705Z 
2025-03-17T21:19:15.3450243Z 2025-03-17T21:19:15.300698896Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (Embedded then 4.1.10.0) STARTED
2025-03-17T21:19:15.5448258Z 2025-03-17T21:19:15.460488896Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (Embedded then 4.1.10.0) SUCCESS (160ms)
2025-03-17T21:19:15.5448892Z 
2025-03-17T21:19:15.5449378Z 2025-03-17T21:19:15.461002183Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (Embedded then 4.1.10.0) WITH {optionForceContinuation=true} STARTED
2025-03-17T21:19:15.6448180Z 2025-03-17T21:19:15.636162199Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (Embedded then 4.1.10.0) WITH {optionForceContinuation=true} SUCCESS (175ms)
2025-03-17T21:19:15.6448906Z 
2025-03-17T21:19:15.6449269Z 2025-03-17T21:19:15.636672871Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (4.1.10.0 then Embedded) STARTED
2025-03-17T21:19:15.8448228Z 2025-03-17T21:19:15.779813140Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (4.1.10.0 then Embedded) SUCCESS (143ms)
2025-03-17T21:19:15.8448824Z 
2025-03-17T21:19:15.8449566Z 2025-03-17T21:19:15.780267967Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (4.1.10.0 then Embedded) WITH {optionForceContinuation=true} STARTED
2025-03-17T21:19:15.9448248Z 2025-03-17T21:19:15.941185049Z YamlIntegrationTests > showcasingTests(Runner) > MultiServer (4.1.10.0 then Embedded) WITH {optionForceContinuation=true} SUCCESS (161ms)

vs

2025-03-17T21:19:14.4449615Z 2025-03-17T21:19:14.375445846Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(Embedded STARTED)
2025-03-17T21:19:14.5447904Z 2025-03-17T21:19:14.496453802Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(Embedded SUCCESS (121ms))
2025-03-17T21:19:14.5448575Z
2025-03-17T21:19:14.5449089Z 2025-03-17T21:19:14.496870499Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(JDBC In-Process STARTED)
2025-03-17T21:19:14.6447848Z 2025-03-17T21:19:14.626601851Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(JDBC In-Process SUCCESS (130ms))
2025-03-17T21:19:14.6448569Z
2025-03-17T21:19:14.6450225Z 2025-03-17T21:19:14.627089079Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (Embedded then !current_version) STARTED)
2025-03-17T21:19:14.8448278Z 2025-03-17T21:19:14.793403399Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (Embedded then !current_version) SUCCESS (166ms))
2025-03-17T21:19:14.8448824Z
2025-03-17T21:19:14.8449315Z 2025-03-17T21:19:14.794018940Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (Embedded then !current_version) WITH {optionForceContinuation=true} STARTED)
2025-03-17T21:19:15.0448535Z 2025-03-17T21:19:14.964409459Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (Embedded then !current_version) WITH {optionForceContinuation=true} SUCCESS (171ms))
2025-03-17T21:19:15.0449586Z
2025-03-17T21:19:15.0450027Z 2025-03-17T21:19:14.964792102Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (!current_version then Embedded) STARTED)
2025-03-17T21:19:15.1448164Z 2025-03-17T21:19:15.119398097Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (!current_version then Embedded) SUCCESS (156ms))
2025-03-17T21:19:15.1448728Z
2025-03-17T21:19:15.1449230Z 2025-03-17T21:19:15.119984738Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (!current_version then Embedded) WITH {optionForceContinuation=true} STARTED)
2025-03-17T21:19:15.3448437Z 2025-03-17T21:19:15.300290885Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (!current_version then Embedded) WITH {optionForceContinuation=true} SUCCESS (180ms))
2025-03-17T21:19:15.3449705Z
2025-03-17T21:19:15.3450243Z 2025-03-17T21:19:15.300698896Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (Embedded then 4.1.10.0) STARTED)
2025-03-17T21:19:15.5448258Z 2025-03-17T21:19:15.460488896Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (Embedded then 4.1.10.0) SUCCESS (160ms))
2025-03-17T21:19:15.5448892Z
2025-03-17T21:19:15.5449378Z 2025-03-17T21:19:15.461002183Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (Embedded then 4.1.10.0) WITH {optionForceContinuation=true} STARTED)
2025-03-17T21:19:15.6448180Z 2025-03-17T21:19:15.636162199Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (Embedded then 4.1.10.0) WITH {optionForceContinuation=true} SUCCESS (175ms))
2025-03-17T21:19:15.6448906Z
2025-03-17T21:19:15.6449269Z 2025-03-17T21:19:15.636672871Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (4.1.10.0 then Embedded) STARTED)
2025-03-17T21:19:15.8448228Z 2025-03-17T21:19:15.779813140Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (4.1.10.0 then Embedded) SUCCESS (143ms))
2025-03-17T21:19:15.8448824Z
2025-03-17T21:19:15.8449566Z 2025-03-17T21:19:15.780267967Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (4.1.10.0 then Embedded) WITH {optionForceContinuation=true} STARTED)
2025-03-17T21:19:15.9448248Z 2025-03-17T21:19:15.941185049Z YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (4.1.10.0 then Embedded) WITH {optionForceContinuation=true} SUCCESS (161ms))

Similarly a failed test in the summary might be something like:

❌ YamlIntegrationTests > showcasingTests(Runner) > MultiServer (4.1.10.0 then Embedded) WITH {optionForceContinuation=true}

vs.

❌ YamlIntegrationTests > showcasingTests(Runner) > showcasingTests(MultiServer (4.1.10.0 then Embedded) WITH {optionForceContinuation=true}

The html test report is worse though, if you look at the failing tests, it does not include the method name, here is a parameterized test from a failed nightly run:
image
Following the link also doesn't add the method name, you have to find it via the stack trace....

Not including the method name is aligned with the default behavior of other junit test template annotations such as @ParameterizedTest and @RepeatedTest, so it seems like aligning our reporting with this behavior seems worthwhile.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

* handle complex test hierarchies. In the record layer we maintain the full hierarchy in the output, so this is not
* necessary, but if integrating some other tools this might be necessary.
*/
public YamlTestExtension(@Nullable final String clusterFile, final boolean includeMethodInDescriptions) {
this.clusterFile = clusterFile;
this.includeMethodInDescriptions = includeMethodInDescriptions;
}

@Override
Expand Down Expand Up @@ -184,34 +194,36 @@ public boolean supportsTestTemplate(final ExtensionContext context) {
@Override
public Stream<TestTemplateInvocationContext> provideTestTemplateInvocationContexts(final ExtensionContext context) {
final var testClass = context.getRequiredTestClass();
final var testMethod = context.getRequiredTestMethod();
if (testClass.getAnnotation(MaintainYamlTestConfig.class) != null) {
final var annotation = testClass.getAnnotation(MaintainYamlTestConfig.class);
return provideInvocationContextsForMaintenance(annotation);
return provideInvocationContextsForMaintenance(annotation, testMethod.getName());
}
final var testMethod = context.getRequiredTestMethod();
if (testMethod.getAnnotation(ExcludeYamlTestConfig.class) != null) {
// excluded tests are still included as configs so that they show up in the test run as skipped, rather than
// just not being there. This may waste some resources if all the tests being run exclude a config that has
// expensive @BeforeAll.
final var annotation = testMethod.getAnnotation(ExcludeYamlTestConfig.class);
return testConfigs
.stream()
.map(config -> new Context(config, annotation.reason(), annotation.value()));
.map(config -> new Context(config, annotation.reason(), annotation.value(),
includeMethodInDescriptions, testMethod.getName()));
} else if (testMethod.getAnnotation(MaintainYamlTestConfig.class) != null) {
final var annotation =
testMethod.getAnnotation(MaintainYamlTestConfig.class);
return provideInvocationContextsForMaintenance(annotation);
return provideInvocationContextsForMaintenance(annotation, testMethod.getName());
}
return testConfigs
.stream()
.map(config -> new Context(config, "", null));
.map(config -> new Context(config, "", null, includeMethodInDescriptions, testMethod.getName()));
}

private Stream<TestTemplateInvocationContext> provideInvocationContextsForMaintenance(@Nonnull final MaintainYamlTestConfig annotation) {
private Stream<TestTemplateInvocationContext> provideInvocationContextsForMaintenance(
@Nonnull final MaintainYamlTestConfig annotation, @Nonnull final String methodName) {
return maintainConfigs
.stream()
.map(config -> new Context(config, "maintenance not needed",
Objects.requireNonNull(annotation.value())));
Objects.requireNonNull(annotation.value()), includeMethodInDescriptions, methodName));
}

/**
Expand All @@ -225,17 +237,27 @@ private static class Context implements TestTemplateInvocationContext {
private final String excludedReason;
@Nullable
private final YamlTestConfigFilters configFilters;
private final boolean includeMethodInDescriptions;
@Nonnull
private final String methodName;

public Context(@Nonnull final YamlTestConfig config, @Nonnull final String excludedReason,
@Nullable final YamlTestConfigFilters configFilters) {
@Nullable final YamlTestConfigFilters configFilters,
final boolean includeMethodInDescriptions, @Nonnull final String methodName) {
this.config = config;
this.excludedReason = excludedReason;
this.configFilters = configFilters;
this.includeMethodInDescriptions = includeMethodInDescriptions;
this.methodName = methodName;
}

@Override
public String getDisplayName(int invocationIndex) {
return config.toString();
if (includeMethodInDescriptions) {
return methodName + "(" + config + ")";
} else {
return config.toString();
}
}

@Override
Expand Down