From 23c6c258f0d17bf78e7171525b1b90a6a180eb7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20Br=C3=BCnings?= Date: Thu, 29 Sep 2022 15:14:36 +0200 Subject: [PATCH] WIP refactor to CleanupMode --- .../spockframework/builder/BuilderHelper.java | 9 ++ .../extension/builtin/TempDirExtension.java | 56 +++++++++-- .../extension/builtin/TempDirInterceptor.java | 94 ++++++++++++++++--- .../runtime/model/MethodKind.java | 25 ++--- .../tempdir/TempDirConfiguration.java | 15 ++- .../src/main/java/spock/lang/TempDir.java | 25 ++++- .../extension/TempDirExtensionSpec.groovy | 59 ++++++++++-- 7 files changed, 240 insertions(+), 43 deletions(-) diff --git a/spock-core/src/main/java/org/spockframework/builder/BuilderHelper.java b/spock-core/src/main/java/org/spockframework/builder/BuilderHelper.java index 51270763d2..d46562b4ab 100644 --- a/spock-core/src/main/java/org/spockframework/builder/BuilderHelper.java +++ b/spock-core/src/main/java/org/spockframework/builder/BuilderHelper.java @@ -18,6 +18,7 @@ import org.codehaus.groovy.runtime.MetaClassHelper; import org.spockframework.runtime.GroovyRuntimeUtil; +import org.spockframework.util.CollectionUtil; public class BuilderHelper { public static Object createInstance(Class clazz, Object... args) { @@ -31,6 +32,14 @@ public static Object createInstance(Class clazz, Object... args) { // IDEA: could support creation of collection types here + if (clazz.isEnum()) { + if (args.length == 1 && args[0] instanceof String) { + return Enum.valueOf(clazz, (String) args[0]); + } else { + throw new IllegalArgumentException("Cannot create enum " + clazz.getName() + " with arguments " + args); + } + } + if ((clazz.getModifiers() & Modifier.ABSTRACT) != 0) { String kind = clazz.isPrimitive() ? "primitive" : clazz.isInterface() ? "interface" : "abstract"; throw new RuntimeException(String.format( "Cannot instantiate %s type %s", kind, clazz.getName())); diff --git a/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TempDirExtension.java b/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TempDirExtension.java index 5e36e71a08..0d09eaaa83 100644 --- a/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TempDirExtension.java +++ b/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TempDirExtension.java @@ -1,9 +1,27 @@ +/* + * Copyright 2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * https://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + package org.spockframework.runtime.extension.builtin; import org.spockframework.runtime.extension.IAnnotationDrivenExtension; +import org.spockframework.runtime.extension.IMethodInterceptor; import org.spockframework.runtime.model.*; import org.spockframework.tempdir.TempDirConfiguration; -import org.spockframework.util.*; +import org.spockframework.util.Beta; +import org.spockframework.util.Checks; +import org.spockframework.util.UnreachableCodeError; import spock.lang.TempDir; import java.util.EnumSet; @@ -25,21 +43,47 @@ public TempDirExtension(TempDirConfiguration configuration) { @Override public void visitFieldAnnotation(TempDir annotation, FieldInfo field) { - TempDirInterceptor interceptor = TempDirInterceptor.forField(field, configuration.baseDir, !annotation.cleanup() || configuration.keep); + TempDir.CleanupMode cleanupMode = annotation.cleanup(); + cleanupMode = cleanupMode == TempDir.CleanupMode.DEFAULT ? configuration.cleanup : cleanupMode; + TempDirInterceptor interceptor = TempDirInterceptor.forField(field, configuration.baseDir, annotation, cleanupMode); - // attach interceptor SpecInfo specInfo = field.getParent(); if (field.isShared()) { specInfo.getBottomSpec().addSharedInitializerInterceptor(interceptor); } else { specInfo.addInitializerInterceptor(interceptor); } + if (cleanupMode == TempDir.CleanupMode.ON_SUCCESS) { + registerForAllFeatures(specInfo, new TempDirInterceptor.FailureTracker(annotation)); + } + } + + private static void registerForAllFeatures(SpecInfo specInfo, IMethodInterceptor interceptor) { + specInfo.getBottomSpec().getAllFeatures().forEach(featureInfo -> featureInfo.getFeatureMethod().addInterceptor(interceptor)); } @Override public void visitParameterAnnotation(TempDir annotation, ParameterInfo parameter) { - Checks.checkArgument(VALID_METHOD_KINDS.contains(parameter.getParent().getKind()), () -> "@TempDir can only be used on setup, setupSpec or feature method parameters."); - TempDirInterceptor interceptor = TempDirInterceptor.forParameter(parameter, configuration.baseDir, configuration.keep); - parameter.getParent().addInterceptor(interceptor); + TempDir.CleanupMode cleanupMode = annotation.cleanup(); + cleanupMode = cleanupMode == TempDir.CleanupMode.DEFAULT ? configuration.cleanup : cleanupMode; + MethodInfo methodInfo = parameter.getParent(); + Checks.checkArgument(VALID_METHOD_KINDS.contains(methodInfo.getKind()), () -> "@TempDir can only be used on setup, setupSpec or feature method parameters."); + TempDirInterceptor interceptor = TempDirInterceptor.forParameter(parameter, configuration.baseDir, annotation, cleanupMode); + methodInfo.addInterceptor(interceptor); + + if (cleanupMode == TempDir.CleanupMode.ON_SUCCESS) { + TempDirInterceptor.FailureTracker failureTracker = new TempDirInterceptor.FailureTracker(annotation); + switch (methodInfo.getKind()) { + case SETUP: + case SETUP_SPEC: + registerForAllFeatures(methodInfo.getParent(), failureTracker); + break; + case FEATURE: + methodInfo.addInterceptor(failureTracker); + break; + default: + throw new UnreachableCodeError(); + } + } } } diff --git a/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TempDirInterceptor.java b/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TempDirInterceptor.java index 650a19bc1b..8a8ef2dbb6 100644 --- a/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TempDirInterceptor.java +++ b/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TempDirInterceptor.java @@ -1,3 +1,18 @@ +/* + * Copyright 2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * https://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + package org.spockframework.runtime.extension.builtin; import org.codehaus.groovy.runtime.ResourceGroovyMethods; @@ -8,6 +23,7 @@ import org.spockframework.runtime.model.FieldInfo; import org.spockframework.runtime.model.ParameterInfo; import org.spockframework.util.*; +import spock.lang.TempDir; import java.io.File; import java.io.IOException; @@ -17,10 +33,10 @@ import java.nio.file.Path; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; +import java.util.Objects; import java.util.regex.Pattern; import static java.nio.file.FileVisitResult.CONTINUE; -import static org.spockframework.runtime.model.MethodInfo.MISSING_ARGUMENT; /** * @author dqyuan @@ -36,18 +52,21 @@ public class TempDirInterceptor implements IMethodInterceptor { private final IThrowableBiConsumer valueSetter; private final String name; private final Path parentDir; - private final boolean keep; + private final TempDir annotation; + private final TempDir.CleanupMode cleanupMode; private TempDirInterceptor( IThrowableBiConsumer valueSetter, String name, Path parentDir, - boolean keep) { + TempDir annotation, + TempDir.CleanupMode cleanupMode) { this.valueSetter = valueSetter; this.name = name; this.parentDir = parentDir; - this.keep = keep; + this.annotation = annotation; + this.cleanupMode = cleanupMode; } private String dirPrefix(IMethodInvocation invocation) { @@ -88,7 +107,9 @@ protected Path setUp(IMethodInvocation invocation) throws Exception { @Override public void intercept(IMethodInvocation invocation) throws Throwable { Path path = setUp(invocation); - invocation.getStore(NAMESPACE).put(path, new TempDirContainer(path, keep)); + // not super thrilled about identityHashCode, but apparently annotations use field equality and not identity + TempDirContainer old = invocation.getStore(NAMESPACE).put(System.identityHashCode(annotation), new TempDirContainer(path, cleanupMode)); + Checks.checkState(old == null, () -> "Replaced other value: " + old.path); invocation.proceed(); } @@ -149,16 +170,17 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) { } } - static TempDirInterceptor forField(FieldInfo fieldInfo, Path parentDir, boolean keep) { + static TempDirInterceptor forField(FieldInfo fieldInfo, Path parentDir, TempDir annotation, TempDir.CleanupMode cleanupMode) { IThrowableFunction typeMapper = createPathToTypeMapper(fieldInfo.getType()); return new TempDirInterceptor( (invocation, path) -> fieldInfo.writeValue(invocation.getInstance(), typeMapper.apply(path)), fieldInfo.getName(), parentDir, - keep); + annotation, + cleanupMode); } - static TempDirInterceptor forParameter(ParameterInfo parameterInfo, Path parentDir, boolean keep) { + static TempDirInterceptor forParameter(ParameterInfo parameterInfo, Path parentDir, TempDir annotation, TempDir.CleanupMode cleanupMode) { IThrowableFunction typeMapper = createPathToTypeMapper(parameterInfo.getReflection().getType()); return new TempDirInterceptor( (IMethodInvocation invocation, Path path) -> { @@ -166,27 +188,69 @@ static TempDirInterceptor forParameter(ParameterInfo parameterInfo, Path parentD }, parameterInfo.getName(), parentDir, - keep); + annotation, + cleanupMode); + } + + static class FailureTracker implements IMethodInterceptor { + private final TempDir annotation; + + FailureTracker(TempDir annotation) { + this.annotation = annotation; + } + + @Override + public void intercept(IMethodInvocation invocation) throws Throwable { + TempDirContainer tempDirContainer = invocation.getStore(NAMESPACE).get(System.identityHashCode(annotation), TempDirContainer.class); + try { + invocation.proceed(); + } catch (Throwable t) { + tempDirContainer.markFailed(); + throw t; + } + } } static class TempDirContainer implements AutoCloseable { private final Path path; - private final boolean keep; + private final TempDir.CleanupMode cleanupMode; + private volatile boolean failed = false; - TempDirContainer(Path path, boolean keep) { + TempDirContainer(Path path, TempDir.CleanupMode cleanupMode) { this.path = path; - this.keep = keep; + this.cleanupMode = cleanupMode; + } + + void markFailed() { + failed = true; + } + + private void destroy(Path path) throws IOException { + switch (cleanupMode) { + case ON_SUCCESS: + if (failed) { + System.err.printf("TempDir '%s' not deleted because the test failed, please delete it manually after investigation.%n", + path.toAbsolutePath()); + return; + } + case ALWAYS: + case DEFAULT: + deleteTempDir(path); + break; + case NEVER: + break; + default: + throw new IllegalStateException("Unknown cleanup mode: " + cleanupMode); + } } @Override public void close() { - if (!keep) { try { - deleteTempDir(path); + destroy(path); } catch (IOException e) { ExceptionUtil.sneakyThrow(e); } - } } } } diff --git a/spock-core/src/main/java/org/spockframework/runtime/model/MethodKind.java b/spock-core/src/main/java/org/spockframework/runtime/model/MethodKind.java index c93eb0fbad..4c39300b4d 100644 --- a/spock-core/src/main/java/org/spockframework/runtime/model/MethodKind.java +++ b/spock-core/src/main/java/org/spockframework/runtime/model/MethodKind.java @@ -1,17 +1,16 @@ /* - * Copyright 2009 the original author or authors. + * Copyright 2024 the original author or authors. * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * https://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. */ package org.spockframework.runtime.model; @@ -54,4 +53,8 @@ public boolean isFeatureScopedFixtureMethod() { public boolean isSpecScopedFixtureMethod() { return this == SETUP_SPEC || this == CLEANUP_SPEC; } + + public boolean isInitializerMethod() { + return this == INITIALIZER || this == SHARED_INITIALIZER; + } } diff --git a/spock-core/src/main/java/org/spockframework/tempdir/TempDirConfiguration.java b/spock-core/src/main/java/org/spockframework/tempdir/TempDirConfiguration.java index 012e2eb9f4..564d6d2131 100644 --- a/spock-core/src/main/java/org/spockframework/tempdir/TempDirConfiguration.java +++ b/spock-core/src/main/java/org/spockframework/tempdir/TempDirConfiguration.java @@ -2,8 +2,10 @@ import org.spockframework.util.Beta; import spock.config.ConfigurationObject; +import spock.lang.TempDir; import java.nio.file.Path; +import java.util.Optional; /** * @@ -32,8 +34,15 @@ public class TempDirConfiguration { public Path baseDir = null; /** - * Whether to keep the temp directory or not after test, - * default is system property {@code spock.tempDir.keep} or false if it is not set. + * Whether to keep the temp directory or not after test if it failed, + * default is system property {@link TempDir#TEMP_DIR_CLEANUP_PROPERTY} or {@link TempDir.CleanupMode#ALWAYS} if it is not set. + * + * @see TempDir#cleanup() + * @see TempDir#TEMP_DIR_CLEANUP_PROPERTY + * + * @since 2.3 */ - public boolean keep = Boolean.getBoolean("spock.tempDir.keep"); + public TempDir.CleanupMode cleanup = Optional.ofNullable(System.getProperty(TempDir.TEMP_DIR_CLEANUP_PROPERTY)) + .map(TempDir.CleanupMode::valueOf) + .orElse(TempDir.CleanupMode.ALWAYS); } diff --git a/spock-core/src/main/java/spock/lang/TempDir.java b/spock-core/src/main/java/spock/lang/TempDir.java index 3f304e9479..c6f12a1461 100644 --- a/spock-core/src/main/java/spock/lang/TempDir.java +++ b/spock-core/src/main/java/spock/lang/TempDir.java @@ -49,8 +49,31 @@ @Target({ElementType.FIELD, ElementType.PARAMETER}) @ExtensionAnnotation(TempDirExtension.class) public @interface TempDir { + String TEMP_DIR_CLEANUP_PROPERTY = "spock.tempdir.cleanup"; + /** * Whether to cleanup the directory after the test. + * + * @since 2.3 */ - boolean cleanup() default true; + CleanupMode cleanup() default CleanupMode.DEFAULT; + + enum CleanupMode { + /** + * Use the default cleanup mode, configured via {@link #TEMP_DIR_CLEANUP_PROPERTY} or {@link org.spockframework.tempdir.TempDirConfiguration#cleanup}. + */ + DEFAULT, + /** + * Always cleanup the directory after the test. + */ + ALWAYS, + /** + * Cleanup the directory only if the test has succeeded. + */ + ON_SUCCESS, + /** + * Never cleanup the directory after the test. + */ + NEVER + } } diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/extension/TempDirExtensionSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/extension/TempDirExtensionSpec.groovy index 5d37805951..5ce800acc0 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/extension/TempDirExtensionSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/extension/TempDirExtensionSpec.groovy @@ -1,3 +1,18 @@ +/* + * Copyright 2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * https://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + package org.spockframework.smoke.extension import org.spockframework.EmbeddedSpecification @@ -84,13 +99,12 @@ class TempDirExtensionSpec extends EmbeddedSpecification { static Path pathFromEmbedded - def "define temp directory location and keep temp directory using configuration script"() { + def "define temp directory location using configuration script"() { given: def userDefinedBase = untypedPath.resolve("build") runner.configurationScript { tempdir { baseDir userDefinedBase - keep true } } runner.addClassImport(Path) @@ -105,18 +119,49 @@ Path temp def method1() { expect: temp.parent.fileName.toString() == "build" +} +""") - cleanup: + then: + result.testsSucceededCount == 1 + } + + def "define cleanup using configuration script"(boolean testSucceeds, TempDir.CleanupMode cleanupMode) { + given: + runner.configurationScript { + tempdir { + cleanup cleanupMode + } + } + runner.addClassImport(Path) + runner.addClassImport(getClass()) + runner.throwFailure = false + + when: + def result = runner.runSpecBody(""" + +@TempDir +Path temp + +def method1() { + given: TempDirExtensionSpec.pathFromEmbedded = temp + + expect: + $testSucceeds } """) then: - result.testsSucceededCount == 1 - Files.exists(pathFromEmbedded) + result.testsSucceededCount == (testSucceeds ? 1 : 0) + result.testsFailedCount == (testSucceeds ? 0 : 1) + Files.exists(pathFromEmbedded) == (!testSucceeds && cleanupMode == TempDir.CleanupMode.ON_SUCCESS) || cleanupMode == TempDir.CleanupMode.NEVER cleanup: - Files.delete(pathFromEmbedded) + Files.deleteIfExists(pathFromEmbedded) + + where: + [testSucceeds, cleanupMode] << [[true, false], TempDir.CleanupMode.values()].combinations() } def "cleanup of files can be disabled"() { @@ -127,7 +172,7 @@ def method1() { when: def result = runner.runSpecBody(""" -@TempDir(cleanup = false) +@TempDir(cleanup = TempDir.CleanupMode.NEVER) Path temp def method1() {