From 55083fe835111c3914eab0b15d74d9b804f14cb3 Mon Sep 17 00:00:00 2001 From: Kevin Cooney Date: Sat, 31 Jan 2026 21:55:06 -0800 Subject: [PATCH] Migrate remaining JUnit4-style tests in lib to JUnit Jupiter --- lib/build.gradle | 6 +- .../preferences/IsolatedPreferences.java | 81 -------------- .../PersistedConfigurationTest.java | 104 +++++++++--------- .../lib2813/util/InputValidationTest.java | 12 +- testing/build.gradle | 6 +- vision/build.gradle | 2 - 6 files changed, 63 insertions(+), 148 deletions(-) delete mode 100644 lib/src/test/java/com/team2813/lib2813/preferences/IsolatedPreferences.java diff --git a/lib/build.gradle b/lib/build.gradle index d1338351..0243d65d 100644 --- a/lib/build.gradle +++ b/lib/build.gradle @@ -21,8 +21,6 @@ dependencies { testImplementation('org.mockito:mockito-core:5.14.2') testImplementation('com.google.truth:truth:1.4.4') testRuntimeOnly('org.junit.platform:junit-platform-launcher') - testRuntimeOnly('org.junit.vintage:junit-vintage-engine') - testImplementation 'junit:junit:4.13.2' testImplementation project(':testing') compileOnly 'com.google.auto.value:auto-value-annotations:1.11.0' annotationProcessor 'com.google.auto.value:auto-value:1.11.0' @@ -31,10 +29,8 @@ dependencies { // the magic line that makes tests work :) wpi.java.configureTestTasks(test) -tasks.named('test') { - // Support running both JUnit Vintage and JUnit Jupiter tests +test { useJUnitPlatform() - systemProperty 'junit.jupiter.extensions.autodetection.enabled', 'true' } mavenPublishing { diff --git a/lib/src/test/java/com/team2813/lib2813/preferences/IsolatedPreferences.java b/lib/src/test/java/com/team2813/lib2813/preferences/IsolatedPreferences.java deleted file mode 100644 index d183769f..00000000 --- a/lib/src/test/java/com/team2813/lib2813/preferences/IsolatedPreferences.java +++ /dev/null @@ -1,81 +0,0 @@ -/* -Copyright 2025-2026 Prospect Robotics SWENext Club - -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 - -http://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 com.team2813.lib2813.preferences; - -import edu.wpi.first.networktables.NetworkTable; -import edu.wpi.first.networktables.NetworkTableInstance; -import edu.wpi.first.networktables.NetworkTableListener; -import edu.wpi.first.wpilibj.Preferences; -import java.lang.reflect.Field; -import org.junit.rules.ExternalResource; - -/** - * A JUnit rule that ensures that changes to preferences done by a test are not leaked out to other - * tests. - */ -final class IsolatedPreferences extends ExternalResource { - private NetworkTableInstance tempInstance; - private NetworkTableInstance prevInstance; - - /** Gets the {@link NetworkTable} that contains the preference values. */ - public NetworkTable getPreferencesTable() { - return tempInstance.getTable("Preferences"); - } - - @Override - protected void before() { - prevInstance = Preferences.getNetworkTable().getInstance(); - tempInstance = NetworkTableInstance.create(); - Preferences.setNetworkTableInstance(tempInstance); - tempInstance.waitForListenerQueue(1); - removePreferencesListener(); - } - - @Override - protected void after() { - Preferences.setNetworkTableInstance(prevInstance); - - // Clear out the listener queue before destroying our temporary NetworkTableInstance. - // - // This works around a race condition in WPILib where a listener registered by Preferences can - // be called after the NetworkTableInstance was closed (see - // https://github.com/wpilibsuite/allwpilib/issues/8215). - if (!tempInstance.waitForListenerQueue(.4)) { - System.err.println( - "Timed out waiting for the NetworkTableInstance listener queue to empty (waited 400ms);" - + " will not close temporary NetworkTableInstance"); - } else { - tempInstance.close(); - } - } - - /** - * Removes the listener installed by {@link - * Preferences#setNetworkTableInstance(NetworkTableInstance)}. - * - *

The listener is a constant source of SIGSEGVs in our GitHub test actions. - */ - private static void removePreferencesListener() { - try { - Field listnerField = Preferences.class.getDeclaredField("m_listener"); - listnerField.setAccessible(true); - NetworkTableListener listener = (NetworkTableListener) listnerField.get(null); - listnerField.set(null, null); - listener.close(); - } catch (NoSuchFieldException | IllegalAccessException | IllegalArgumentException e) { - } - } -} diff --git a/lib/src/test/java/com/team2813/lib2813/preferences/PersistedConfigurationTest.java b/lib/src/test/java/com/team2813/lib2813/preferences/PersistedConfigurationTest.java index 7be1e470..19d1d9fd 100644 --- a/lib/src/test/java/com/team2813/lib2813/preferences/PersistedConfigurationTest.java +++ b/lib/src/test/java/com/team2813/lib2813/preferences/PersistedConfigurationTest.java @@ -19,59 +19,69 @@ import static com.google.common.truth.Truth.assertWithMessage; import static com.team2813.lib2813.preferences.PersistedConfiguration.REGISTERED_CLASSES_NETWORK_TABLE_KEY; import static java.util.stream.Collectors.toMap; -import static org.junit.Assert.assertThrows; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; +import com.team2813.lib2813.testing.junit.jupiter.ProvideUniqueNetworkTableInstance; import edu.wpi.first.networktables.NetworkTable; import edu.wpi.first.networktables.NetworkTableEntry; +import edu.wpi.first.networktables.NetworkTableInstance; import edu.wpi.first.networktables.NetworkTableType; import edu.wpi.first.networktables.Topic; import edu.wpi.first.wpilibj.DataLogManager; import edu.wpi.first.wpilibj.Preferences; +import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.*; -import org.junit.After; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.experimental.runners.Enclosed; -import org.junit.rules.ErrorCollector; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameters; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.params.ParameterizedClass; +import org.junit.jupiter.params.provider.ValueSource; /** Tests for {@link PersistedConfiguration}. */ -@RunWith(Enclosed.class) +@ProvideUniqueNetworkTableInstance(replacePreferencesNetworkTable = true) public final class PersistedConfigurationTest { private static final double EPSILON = 0.001; /** Base class for all nested classes of {@link PersistedConfigurationTest}. */ - abstract static class PreferencesRegistryTestCase { + @Nested + abstract class PreferencesRegistryTestCase { + private final List collectedErrors = new ArrayList<>(); private final String preferenceName; private final Class recordClass; - - @Rule public final IsolatedPreferences isolatedPreferences = new IsolatedPreferences(); - @Rule public final ErrorCollector errorCollector = new ErrorCollector(); + private NetworkTableInstance ntInstance; + private NetworkTable preferencesTable; protected PreferencesRegistryTestCase(String preferenceName, Class recordClass) { this.preferenceName = preferenceName; this.recordClass = recordClass; } - @Before + @BeforeEach + public final void injectNetworkTableInstance(NetworkTableInstance ntInstance) { + this.ntInstance = ntInstance; + preferencesTable = ntInstance.getTable("Preferences"); + } + + @BeforeEach public final void setTestGlobals() { PersistedConfiguration.throwExceptions = true; PersistedConfiguration.errorReporter = - message -> - errorCollector.addError( - new AssertionError("Unexpected warning: \"" + message + "\"")); + message -> collectedErrors.add(() -> fail("Unexpected warning: \"" + message + "\"")); } - @After + @AfterEach public final void resetTestGlobals() { PersistedConfiguration.throwExceptions = false; PersistedConfiguration.errorReporter = DataLogManager::log; + assertAll(collectedErrors); } protected enum ValuesKind { @@ -80,7 +90,7 @@ protected enum ValuesKind { } private NetworkTableEntry getTableEntry(String key, NetworkTableType expectedType) { - NetworkTableEntry entry = isolatedPreferences.getPreferencesTable().getEntry(key); + NetworkTableEntry entry = preferencesTable.getEntry(key); assertThat(entry.getType()).isEqualTo(expectedType); return entry; } @@ -102,8 +112,7 @@ protected final String getStringValue(String key) { } protected final void setIntegerValue(String key, int value) { - NetworkTable table = isolatedPreferences.getPreferencesTable(); - NetworkTableEntry entry = table.getEntry(key); + NetworkTableEntry entry = preferencesTable.getEntry(key); entry.setInteger(value); entry.setPersistent(); } @@ -116,15 +125,16 @@ protected final void assertHasNoChangesSince(Map previousValues) } protected final Map preferenceValues() { - NetworkTable table = isolatedPreferences.getPreferencesTable(); return preferenceKeys().stream() - .collect(toMap(Function.identity(), key -> table.getEntry(key).getValue().getValue())); + .collect( + toMap( + Function.identity(), + key -> preferencesTable.getEntry(key).getValue().getValue())); } protected final Set preferenceKeys() { - NetworkTable table = isolatedPreferences.getPreferencesTable(); Set keys = new HashSet<>(); - collectKeys(table, keys); + collectKeys(preferencesTable, keys); return Set.copyOf(keys); } @@ -200,11 +210,7 @@ public void preferenceNameMapsToOnlyOneRecordType() { .containsMatch("Preference with name '" + preferenceName + "' already registered"); // Assert: topic added under "/PersistedConfiguration", and is not persistent - NetworkTable table = - isolatedPreferences - .getPreferencesTable() - .getInstance() - .getTable(REGISTERED_CLASSES_NETWORK_TABLE_KEY); + NetworkTable table = ntInstance.getTable(REGISTERED_CLASSES_NETWORK_TABLE_KEY); NetworkTableEntry entry = table.getEntry(preferenceName); assertThat(entry.exists()).isTrue(); assertThat(entry.isPersistent()).isFalse(); @@ -340,8 +346,10 @@ public void withExistingPreferences_passingRecordClass() { } } - @RunWith(Parameterized.class) - public static class BooleanPreferencesTest + @Nested + @ParameterizedClass(name = "defaultValue={0}") + @ValueSource(booleans = {true, false}) + public class BooleanPreferencesTest extends PreferencesRegistryTestCase { static final String PREFERENCE_NAME = "Booleans"; static final String BOOLEAN_VALUE_KEY = "Booleans/booleanValue"; @@ -349,11 +357,6 @@ public static class BooleanPreferencesTest static final Set ALL_KEYS = Set.of(BOOLEAN_VALUE_KEY, BOOLEAN_SUPPLIER_KEY); final boolean defaultValue; - @Parameters(name = "defaultValue={0}") - public static Object[] data() { - return new Object[] {true, false}; - } - public BooleanPreferencesTest(boolean defaultValue) { super(PREFERENCE_NAME, RecordWithBooleans.class); this.defaultValue = defaultValue; @@ -429,19 +432,16 @@ protected void assertSuppliersHaveUpdatedValues(RecordWithBooleans record) { } } - @RunWith(Parameterized.class) - public static class IntPreferencesTest + @Nested + @ParameterizedClass(name = "storeAsDoubles={0}") + @ValueSource(booleans = {true, false}) + public class IntPreferencesTest extends PreferencesRegistryTestCase { static final String PREFERENCE_NAME = "Integers"; static final String INT_VALUE_KEY = "Integers/intValue"; static final String INT_SUPPLIER_KEY = "Integers/intSupplier"; final boolean storeAsDoubles; - @Parameters(name = "storeAsDoubles={0}") - public static Object[] data() { - return new Object[] {true, false}; - } - public IntPreferencesTest(boolean storeAsDoubles) { super(PREFERENCE_NAME, RecordWithInts.class); this.storeAsDoubles = storeAsDoubles; @@ -533,7 +533,8 @@ protected void assertSuppliersHaveUpdatedValues(RecordWithInts record) { } } - public static class LongPreferencesTest + @Nested + public class LongPreferencesTest extends PreferencesRegistryTestCase { static final String PREFERENCE_NAME = "Longs"; static final String LONG_VALUE_KEY = "Longs/longValue"; @@ -616,7 +617,8 @@ protected void assertSuppliersHaveUpdatedValues(RecordWithLongs record) { } } - public static class DoublePreferencesTest + @Nested + public class DoublePreferencesTest extends PreferencesRegistryTestCase { static final String PREFERENCE_NAME = "Doubles"; static final String DOUBLE_VALUE_KEY = "Doubles/doubleValue"; @@ -699,7 +701,8 @@ protected void assertSuppliersHaveUpdatedValues(RecordWithDoubles record) { } } - public static class StringPreferencesTest + @Nested + public class StringPreferencesTest extends PreferencesRegistryTestCase { static final String PREFERENCE_NAME = "Strings"; static final String STRING_VALUE_KEY = "Strings/stringValue"; @@ -781,7 +784,8 @@ protected void assertSuppliersHaveUpdatedValues(RecordWithStrings record) { } } - public static class RecordPreferencesTest + @Nested + public class RecordPreferencesTest extends PreferencesRegistryTestCase { static final String PREFERENCE_NAME = "Records"; static final String recordValueKey = "Records/recordValue"; diff --git a/lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java b/lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java index 0670488e..536410b8 100644 --- a/lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java +++ b/lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java @@ -1,5 +1,5 @@ /* -Copyright 2023-2025 Prospect Robotics SWENext Club +Copyright 2023-2026 Prospect Robotics SWENext Club Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -19,14 +19,14 @@ import static com.google.common.truth.Truth.assertWithMessage; import static org.junit.Assert.assertThrows; -import org.junit.Test; -import org.junit.experimental.runners.Enclosed; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; -@RunWith(Enclosed.class) public class InputValidationTest { + // Tests for the `InputValidation.checkCanId(...)` method. - public static class CheckCanIdTest { + @Nested + public class CheckCanIdTest { @Test public void invalidCanId() { // Can IDs can only valid in the range [0, 62]. diff --git a/testing/build.gradle b/testing/build.gradle index 97419232..7be88741 100644 --- a/testing/build.gradle +++ b/testing/build.gradle @@ -26,12 +26,10 @@ dependencies { // the magic line that makes tests work :) wpi.java.configureTestTasks(test) -tasks.named('test') { - // Support running both JUnit Vintage and JUnit Jupiter tests +test { useJUnitPlatform { - excludeTags('ignore-outside-testkit') + excludeTags 'ignore-outside-testkit' } - systemProperty 'junit.jupiter.extensions.autodetection.enabled', 'true' } mavenPublishing { diff --git a/vision/build.gradle b/vision/build.gradle index 3f79748e..9d73eaaa 100644 --- a/vision/build.gradle +++ b/vision/build.gradle @@ -14,7 +14,6 @@ dependencies { testImplementation 'com.google.truth:truth:1.4.4' testImplementation project(':testing') testRuntimeOnly('org.junit.platform:junit-platform-launcher') - testRuntimeOnly('org.junit.vintage:junit-vintage-engine') nativeDebug wpi.java.deps.wpilibJniDebug(wpi.platforms.desktop) nativeDebug wpi.java.vendor.jniDebug(wpi.platforms.desktop) @@ -29,7 +28,6 @@ wpi.java.configureTestTasks(test) test { useJUnitPlatform() - systemProperty 'junit.jupiter.extensions.autodetection.enabled', 'true' } mavenPublishing {