Skip to content

Commit

Permalink
Merge branch 'main' into leo/fix-for-join-pool-thread-deadlock
Browse files Browse the repository at this point in the history
  • Loading branch information
marcphilipp committed Sep 16, 2024
2 parents 71ecec3 + da8eb5f commit 063af2d
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 27 deletions.
2 changes: 1 addition & 1 deletion .github/actions/run-gradle/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ runs:
distribution: temurin
java-version: 21
check-latest: true
- uses: gradle/actions/setup-gradle@16bf8bc8fe830fa669c3c9f914d3eb147c629707 # v4
- uses: gradle/actions/setup-gradle@d156388eb19639ec20ade50009f3d199ce1e2808 # v4
- shell: bash
env:
JAVA_HOME: ${{ steps.setup-gradle-jdk.outputs.path }}
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
- name: Check out repository
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4
- name: Initialize CodeQL
uses: github/codeql-action/init@4dd16135b69a43b6c8efb853346f8437d92d3c93 # v3
uses: github/codeql-action/init@8214744c546c1e5c8f03dde8fab3a7353211988d # v3
with:
languages: ${{ matrix.language }}
tools: latest
Expand All @@ -44,4 +44,4 @@ jobs:
-Dscan.tag.CodeQL \
allMainClasses
- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@4dd16135b69a43b6c8efb853346f8437d92d3c93 # v3
uses: github/codeql-action/analyze@8214744c546c1e5c8f03dde8fab3a7353211988d # v3
2 changes: 1 addition & 1 deletion .github/workflows/gradle-dependency-submission.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ jobs:
java-version: 21
check-latest: true
- name: Generate and submit dependency graph
uses: gradle/actions/dependency-submission@16bf8bc8fe830fa669c3c9f914d3eb147c629707 # v4
uses: gradle/actions/dependency-submission@d156388eb19639ec20ade50009f3d199ce1e2808 # v4
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ on GitHub.

* Fixed potential locking issue with `ExclusiveResource` in the
`HierarchicalTestExecutorService`, which could lead to deadlocks in certain scenarios.
* `NamespacedHierarchicalStore` no longer throws an exception after it has been closed if
the store is queried via one of the `get(...)` or `getOrComputeIfAbsent(...)` methods;
however, if a `getOrComputeIfAbsent(...)` invocation results in the computation of a new
value, an exception will still be thrown.

[[release-notes-5.11.1-junit-platform-deprecations-and-breaking-changes]]
==== Deprecations and Breaking Changes
Expand All @@ -37,7 +41,8 @@ on GitHub.
[[release-notes-5.11.1-junit-jupiter-bug-fixes]]
==== Bug Fixes

* ❓
* `TestWatcher` callback methods can once again access data in the
`ExtensionContext.Store`.

[[release-notes-5.11.1-junit-jupiter-deprecations-and-breaking-changes]]
==== Deprecations and Breaking Changes
Expand Down
8 changes: 4 additions & 4 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ asciidoctor-plugins = "4.0.3" # Check if workaround in documentation.gradle.kts
assertj = "3.26.3"
bnd = "7.0.0"
checkstyle = "10.18.1"
eclipse = "4.32.0"
eclipse = "4.33.0"
jackson = "2.17.2"
jacoco = "0.8.12"
jmh = "1.37"
Expand Down Expand Up @@ -34,7 +34,7 @@ bndlib = { module = "biz.aQute.bnd:biz.aQute.bndlib", version.ref = "bnd" }
checkstyle = { module = "com.puppycrawl.tools:checkstyle", version.ref = "checkstyle" }
classgraph = { module = "io.github.classgraph:classgraph", version = "4.8.175" }
commons-io = { module = "commons-io:commons-io", version = "2.16.1" }
groovy4 = { module = "org.apache.groovy:groovy", version = "4.0.22" }
groovy4 = { module = "org.apache.groovy:groovy", version = "4.0.23" }
groovy2-bom = { module = "org.codehaus.groovy:groovy-bom", version = "2.5.23" }
hamcrest = { module = "org.hamcrest:hamcrest", version = "3.0" }
jackson-dataformat-yaml = { module = "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml", version.ref = "jackson" }
Expand All @@ -47,7 +47,7 @@ jmh-generator-annprocess = { module = "org.openjdk.jmh:jmh-generator-annprocess"
joox = { module = "org.jooq:joox", version = "2.0.1" }
jte = { module = "gg.jte:jte", version = "3.1.12" }
junit4 = { module = "junit:junit", version = { require = "[4.12,)", prefer = "4.13.2" } }
kotlinx-coroutines = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version = "1.8.1" }
kotlinx-coroutines = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version = "1.9.0" }
log4j-core = { module = "org.apache.logging.log4j:log4j-core", version.ref = "log4j" }
log4j-jul = { module = "org.apache.logging.log4j:log4j-jul", version.ref = "log4j" }
maven = { module = "org.apache.maven:apache-maven", version = "3.9.9" }
Expand Down Expand Up @@ -90,7 +90,7 @@ foojayResolver = { id = "org.gradle.toolchains.foojay-resolver", version = "0.8.
gitPublish = { id = "org.ajoberstar.git-publish", version = "4.2.2" }
jmh = { id = "me.champeau.jmh", version = "0.7.2" }
nexusPublish = { id = "io.github.gradle-nexus.publish-plugin", version = "2.0.0" }
plantuml = { id = "io.freefair.plantuml", version = "8.6" }
plantuml = { id = "io.freefair.plantuml", version = "8.10" }
shadow = { id = "com.gradleup.shadow", version = "8.3.1" }
spotless = { id = "com.diffplug.spotless", version = "7.0.0.BETA2" }
versions = { id = "com.github.ben-manes.versions", version = "0.51.0" }
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ public void close() {
* closed
*/
public Object get(N namespace, Object key) {
rejectIfClosed();
StoredValue storedValue = getStoredValue(new CompositeKey<>(namespace, key));
return StoredValue.evaluateIfNotNull(storedValue);
}
Expand All @@ -156,7 +155,6 @@ public Object get(N namespace, Object key) {
* be cast to the required type, or if this store has already been closed
*/
public <T> T get(N namespace, Object key, Class<T> requiredType) throws NamespacedHierarchicalStoreException {
rejectIfClosed();
Object value = get(namespace, key);
return castToRequiredType(key, value, requiredType);
}
Expand All @@ -174,13 +172,15 @@ public <T> T get(N namespace, Object key, Class<T> requiredType) throws Namespac
* closed
*/
public <K, V> Object getOrComputeIfAbsent(N namespace, K key, Function<K, V> defaultCreator) {
rejectIfClosed();
Preconditions.notNull(defaultCreator, "defaultCreator must not be null");
CompositeKey<N> compositeKey = new CompositeKey<>(namespace, key);
StoredValue storedValue = getStoredValue(compositeKey);
if (storedValue == null) {
storedValue = this.storedValues.computeIfAbsent(compositeKey,
__ -> storedValue(new MemoizingSupplier(() -> defaultCreator.apply(key))));
__ -> storedValue(new MemoizingSupplier(() -> {
rejectIfClosed();
return defaultCreator.apply(key);
})));
}
return storedValue.evaluate();
}
Expand All @@ -202,7 +202,6 @@ public <K, V> Object getOrComputeIfAbsent(N namespace, K key, Function<K, V> def
public <K, V> V getOrComputeIfAbsent(N namespace, K key, Function<K, V> defaultCreator, Class<V> requiredType)
throws NamespacedHierarchicalStoreException {

rejectIfClosed();
Object value = getOrComputeIfAbsent(namespace, key, defaultCreator);
return castToRequiredType(key, value, requiredType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import static java.util.function.Predicate.not;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.entry;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
Expand Down Expand Up @@ -41,8 +42,11 @@
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;
import org.junit.jupiter.api.TestMethodOrder;
import org.junit.jupiter.api.extension.BeforeTestExecutionCallback;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ExtensionContext.Namespace;
import org.junit.jupiter.api.extension.ExtensionContext.Store;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.api.extension.TestWatcher;
import org.junit.jupiter.api.fixtures.TrackLogRecords;
Expand All @@ -67,6 +71,7 @@ class TestWatcherTests extends AbstractJupiterTestEngineTests {
@BeforeEach
void clearResults() {
TrackingTestWatcher.results.clear();
DataRetrievingTestWatcher.results.clear();
}

@Test
Expand Down Expand Up @@ -167,6 +172,16 @@ void testWatcherSemanticsWhenRegisteredAtMethodLevel() {
assertThat(TrackingTestWatcher.results.get("testDisabled")).containsExactly("test", "repeatedTest");
}

@Test
void testWatcherCanRetrieveDataFromTheExtensionContextStore() {
Class<?> testClass = DataRetrievingTestWatcherTestCase.class;
EngineExecutionResults results = executeTestsForClass(testClass);

results.testEvents().assertStatistics(stats -> stats.started(1).succeeded(1).failed(0));

assertThat(DataRetrievingTestWatcher.results).containsExactly(entry("key", "enigma"));
}

private void assertCommonStatistics(EngineExecutionResults results) {
results.containerEvents().assertStatistics(stats -> stats.started(3).succeeded(3).failed(0));
results.testEvents().assertStatistics(stats -> stats.skipped(2).started(6).succeeded(2).aborted(2).failed(2));
Expand Down Expand Up @@ -414,4 +429,40 @@ public void testFailed(ExtensionContext context, Throwable cause) {

}

/**
* {@link TestWatcher} that retrieves data from the {@link ExtensionContext.Store}.
* @see <a href="https://github.com/junit-team/junit5/issues/3944">#3944</a>
*/
static class DataRetrievingTestWatcher implements BeforeTestExecutionCallback, TestWatcher {

private static final Namespace NAMESPACE = Namespace.create(DataRetrievingTestWatcher.class);

private static final String KEY = "key";

private static final Map<String, String> results = new HashMap<>();

@Override
public void beforeTestExecution(ExtensionContext context) throws Exception {
getStore(context).put(KEY, "enigma");
}

@Override
public void testSuccessful(ExtensionContext context) {
results.put(KEY, getStore(context).get(KEY, String.class));
}

private static Store getStore(ExtensionContext context) {
return context.getStore(NAMESPACE);
}
}

@ExtendWith(DataRetrievingTestWatcher.class)
static class DataRetrievingTestWatcherTestCase {

@Test
public void test() {
//no-op
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -519,28 +519,35 @@ void closeIsIdempotent() throws Throwable {
verifyNoMoreInteractions(closeAction);
}

/**
* @see <a href="https://github.com/junit-team/junit5/issues/3944">#3944</a>
*/
@Test
void rejectsModificationAfterClose() {
void acceptsQueryAfterClose() {
store.put(namespace, key, value);
store.close();
assertClosed();

assertThrows(NamespacedHierarchicalStoreException.class, () -> store.put(namespace, "key1", "value1"));
assertThrows(NamespacedHierarchicalStoreException.class, () -> store.remove(namespace, "key1"));
assertThrows(NamespacedHierarchicalStoreException.class,
() -> store.remove(namespace, "key1", Number.class));
assertThat(store.get(namespace, key)).isEqualTo(value);
assertThat(store.get(namespace, key, String.class)).isEqualTo(value);
assertThat(store.getOrComputeIfAbsent(namespace, key, k -> "new")).isEqualTo(value);
assertThat(store.getOrComputeIfAbsent(namespace, key, k -> "new", String.class)).isEqualTo(value);
}

@Test
void rejectsQueryAfterClose() {
void rejectsModificationAfterClose() {
store.close();
assertClosed();

assertThrows(NamespacedHierarchicalStoreException.class, () -> store.get(namespace, "key1"));
assertThrows(NamespacedHierarchicalStoreException.class, () -> store.get(namespace, "key1", Integer.class));
assertThrows(NamespacedHierarchicalStoreException.class, () -> store.put(namespace, key, value));
assertThrows(NamespacedHierarchicalStoreException.class, () -> store.remove(namespace, key));
assertThrows(NamespacedHierarchicalStoreException.class, () -> store.remove(namespace, key, int.class));

// Since key does not exist, an invocation of getOrComputeIfAbsent(...) will attempt to compute a new value.
assertThrows(NamespacedHierarchicalStoreException.class,
() -> store.getOrComputeIfAbsent(namespace, "key1", k -> "value"));
() -> store.getOrComputeIfAbsent(namespace, key, k -> "new"));
assertThrows(NamespacedHierarchicalStoreException.class,
() -> store.getOrComputeIfAbsent(namespace, "key1", k -> 1337, Integer.class));
() -> store.getOrComputeIfAbsent(namespace, key, k -> "new", String.class));
}

private void assertNotClosed() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import org.jetbrains.kotlin.gradle.tasks.KotlinCompile

plugins {
kotlin("jvm") version "1.9.25"
kotlin("jvm") version "2.0.20"
}

repositories {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
</plugin>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<version>2.22.2</version>
<version>3.5.0</version>
</plugin>
</plugins>
</pluginManagement>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
</plugin>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<version>2.22.2</version>
<version>3.5.0</version>
<configuration>
<properties>
<configurationParameters>
Expand Down

0 comments on commit 063af2d

Please sign in to comment.