From 27d3bd232983651d97a3cf60132cda239e2b8c57 Mon Sep 17 00:00:00 2001 From: Dean Brettle Date: Thu, 25 Apr 2024 14:55:41 -0700 Subject: [PATCH 1/2] Add support for overrideable configuration settings such as feature flags. Also modify the CI build task to fail if any settings are overridden. --- .github/workflows/gradle.yml | 2 + .../java/org/carlmontrobotics/Config.java | 95 +++++++++++++++++++ .../org/carlmontrobotics/RobotContainer.java | 4 + .../java/org/carlmontrobotics/ConfigTest.java | 81 ++++++++++++++++ 4 files changed, 182 insertions(+) create mode 100644 src/main/java/org/carlmontrobotics/Config.java create mode 100644 src/test/java/org/carlmontrobotics/ConfigTest.java diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml index 93f64dd1..63171413 100644 --- a/.github/workflows/gradle.yml +++ b/.github/workflows/gradle.yml @@ -17,4 +17,6 @@ jobs: distribution: 'temurin' java-version: 17 - name: Build with Gradle + env: + testCONFIGIsDefault: true run: ./gradlew build diff --git a/src/main/java/org/carlmontrobotics/Config.java b/src/main/java/org/carlmontrobotics/Config.java new file mode 100644 index 00000000..77d168c4 --- /dev/null +++ b/src/main/java/org/carlmontrobotics/Config.java @@ -0,0 +1,95 @@ +package org.carlmontrobotics; + +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.List; + +import edu.wpi.first.util.sendable.Sendable; +import edu.wpi.first.util.sendable.SendableBuilder; + +abstract class Config implements Sendable { + public static final Config CONFIG = new Config() { + { + // Override config settings here, like this: + this.exampleFlagEnabled = true; + } + }; + + // Add additional config settings by declaring a protected field, and... + protected boolean exampleFlagEnabled = false; + + // ...a public getter starting with "is" for booleans or "get" for other types. + // Do NOT remove this example. It is used by unit tests. + public boolean isExampleFlagEnabled() { + return exampleFlagEnabled; + } + + // --- For clarity, place additional config settings ^above^ this line --- + + private static class MethodResult { + String methodName = null; + Object retVal = null; + Object defaultRetVal = null; + + MethodResult(String name, Object retVal, Object defaultRetval) { + this.methodName = name; + this.retVal = retVal; + this.defaultRetVal = defaultRetval; + } + } + + private List getMethodResults() { + var methodResults = new ArrayList(); + var defaultConfig = new Config() { + }; + for (Method m : Config.class.getDeclaredMethods()) { + var name = m.getName(); + if (!Modifier.isPublic(m.getModifiers()) || m.isSynthetic() || m.getParameterCount() != 0 + || !name.matches("^(get|is)[A-Z].*")) { + continue; + } + Object retVal = null; + try { + retVal = m.invoke(this); + } catch (Exception ex) { + retVal = ex; + } + Object defaultRetVal = null; + try { + defaultRetVal = m.invoke(defaultConfig); + } catch (Exception ex) { + defaultRetVal = ex; + } + methodResults.add(new MethodResult(name, retVal, defaultRetVal)); + } + return methodResults; + } + + @Override + public void initSendable(SendableBuilder builder) { + getMethodResults().forEach(mr -> { + if (!mr.retVal.equals(mr.defaultRetVal)) { + builder.publishConstString("%s()".formatted(mr.methodName), + String.format("%s (default is %s)", mr.retVal, mr.defaultRetVal)); + } + }); + } + + @Override + public String toString() { + StringBuilder stringBuilder = new StringBuilder(); + getMethodResults().forEach(mr -> { + if (!mr.retVal.equals(mr.defaultRetVal)) { + stringBuilder.append( + String.format("%s() returns %s (default is %s)", mr.methodName, mr.retVal, mr.defaultRetVal)); + } + }); + if (stringBuilder.isEmpty()) { + stringBuilder.append("Using default config values"); + } else { + stringBuilder.insert(0, "WARNING: USING OVERRIDDEN CONFIG VALUES\n"); + } + return stringBuilder.toString(); + } +} diff --git a/src/main/java/org/carlmontrobotics/RobotContainer.java b/src/main/java/org/carlmontrobotics/RobotContainer.java index 1c4aa461..dc8ab15c 100644 --- a/src/main/java/org/carlmontrobotics/RobotContainer.java +++ b/src/main/java/org/carlmontrobotics/RobotContainer.java @@ -117,6 +117,10 @@ public class RobotContainer { public RobotContainer() { { + // Put any configuration overrides to the dashboard and the terminal + SmartDashboard.putData("CONFIG overrides", Config.CONFIG); + System.out.println(Config.CONFIG); + //safe auto setup... stuff in setupAutos() is not safe to run here - will break robot registerAutoCommands(); SmartDashboard.putData(autoSelector); diff --git a/src/test/java/org/carlmontrobotics/ConfigTest.java b/src/test/java/org/carlmontrobotics/ConfigTest.java new file mode 100644 index 00000000..88689ac9 --- /dev/null +++ b/src/test/java/org/carlmontrobotics/ConfigTest.java @@ -0,0 +1,81 @@ +package org.carlmontrobotics; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; + +import edu.wpi.first.util.sendable.SendableBuilder; +import edu.wpi.first.wpilibj.smartdashboard.SendableBuilderImpl; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.HashMap; + +public class ConfigTest { + @Test + void testIsExampleFlagEnabled() { + assertEquals(false, new Config() { + }.isExampleFlagEnabled()); + assertEquals(true, new Config() { + { + this.exampleFlagEnabled = true; + } + }.isExampleFlagEnabled()); + } + + @Test + void testInitSendable() throws Exception { + var publishedStrings = new HashMap(); + try (SendableBuilder testBuilder = new SendableBuilderImpl() { + @Override + public void publishConstString(String key, String value) { + publishedStrings.put(key, value); + } + }) { + Config testConfig = new Config() { + }; + testConfig.initSendable(testBuilder); + assertEquals(new HashMap(), publishedStrings, + "A default config should not publish anything."); + + testConfig = new Config() { + { + this.exampleFlagEnabled = true; + } + }; + testConfig.initSendable(testBuilder); + assertEquals(new HashMap() { + { + this.put("isExampleFlagEnabled()", "true (default is false)"); + } + }, publishedStrings, "A config with overrides should publish what is overriden."); + } + } + + @Test + public void testToString() { + assertEquals("Using default config values", new Config() { + }.toString()); + assertEquals("WARNING: USING OVERRIDDEN CONFIG VALUES\nisExampleFlagEnabled() returns true (default is false)", + new Config() { + { + exampleFlagEnabled = true; + } + }.toString()); + } + + @Test + @EnabledIfEnvironmentVariable(named = "testCONFIGIsDefault", matches = "true", disabledReason = "not trying to modify GitHub master") + public void testCONFIGIsDefault() throws Exception { + var publishedStrings = new HashMap(); + try (SendableBuilder testBuilder = new SendableBuilderImpl() { + @Override + public void publishConstString(String key, String value) { + publishedStrings.put(key, value); + } + }) { + Config.CONFIG.initSendable(testBuilder); + assertEquals(new HashMap(), publishedStrings, + "Config.CONFIG must be empty to be on the master branch."); + } + } +} From 932d1cf8028d0a0f9d8c2c53fe54a574f4e8dfd7 Mon Sep 17 00:00:00 2001 From: Dean Brettle Date: Thu, 25 Apr 2024 15:08:50 -0700 Subject: [PATCH 2/2] Remove overriden setting so CI passes. Also add some clarifying comments and use a better test name for the test that fails when a setting is overriden. --- src/main/java/org/carlmontrobotics/Config.java | 6 +++++- src/test/java/org/carlmontrobotics/ConfigTest.java | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/carlmontrobotics/Config.java b/src/main/java/org/carlmontrobotics/Config.java index 77d168c4..0d691ee8 100644 --- a/src/main/java/org/carlmontrobotics/Config.java +++ b/src/main/java/org/carlmontrobotics/Config.java @@ -12,7 +12,11 @@ abstract class Config implements Sendable { public static final Config CONFIG = new Config() { { // Override config settings here, like this: - this.exampleFlagEnabled = true; + // this.exampleFlagEnabled = true; + + // NOTE: PRs with overrides will NOT be merged because we don't want them + // polluting the master branch. + // Feel free to add them when testing, but remove them before pushing. } }; diff --git a/src/test/java/org/carlmontrobotics/ConfigTest.java b/src/test/java/org/carlmontrobotics/ConfigTest.java index 88689ac9..3b0575aa 100644 --- a/src/test/java/org/carlmontrobotics/ConfigTest.java +++ b/src/test/java/org/carlmontrobotics/ConfigTest.java @@ -65,7 +65,7 @@ public void testToString() { @Test @EnabledIfEnvironmentVariable(named = "testCONFIGIsDefault", matches = "true", disabledReason = "not trying to modify GitHub master") - public void testCONFIGIsDefault() throws Exception { + public void testNoConfigSettingsOverridden() throws Exception { var publishedStrings = new HashMap(); try (SendableBuilder testBuilder = new SendableBuilderImpl() { @Override