From 5b746be573849765da6efc0e944bbfdc321686ab Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Mon, 22 Jun 2026 18:39:35 -0700 Subject: [PATCH] test: run the unit suite against both validation engines (#467) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To gain confidence before any of this is released, run the whole suite twice — once on the original SyntacticMatch/SemanticMatch engine and once on the new list-of-successes engine — rather than relying on a couple of e2e tests. - GrammarOptionValue.FORCE_PARSE_ENGINE: a system property (-Dsystemd.unit.grammarParseEngine=true) forces validation onto the new engine, independent of the per-project experimental flag. Only validation is forced; the cosmetic annotators (coloring, key marker) stay on the user flag, so problem COUNTS are unchanged and only exact error spans can differ between engines. - build.gradle.kts forwards the property to the forked test JVM. - CI (main.yml) gains a grammarParseEngine: [false, true] matrix dimension, running both engines in parallel. Result: the entire suite passes under BOTH engines. The only test needing engine-aware expectations is InvalidValueInspectionForCGroupSocketBind, where the new engine localizes two errors differently (and arguably better): "ipv6::tcp" highlights ":tcp" after the consumed "ipv6:" rather than "::tcp"; "12--21485" points at the out-of-range port "-21485" rather than the dash. Counts/validity match everywhere else — strong evidence the new engine is at parity. Refs #467 Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/main.yml | 8 ++++++-- build.gradle.kts | 4 ++++ .../optionvalues/grammar/GrammarOptionValue.kt | 12 +++++++++++- .../InvalidValueInspectionForCGroupSocketBind.kt | 9 +++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2572b941..8c478781 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -20,7 +20,10 @@ jobs: matrix: distribution: [ 'temurin' ] java: [ '21' ] - name: Java ${{ matrix.Java }} Build + # Run the unit tests against both the original validation engine and the new + # list-of-successes engine, in parallel. + grammarParseEngine: [ 'false', 'true' ] + name: Java ${{ matrix.java }} Build (grammar engine ${{ matrix.grammarParseEngine }}) steps: - name: Check out Git repository uses: actions/checkout@v7 @@ -46,10 +49,11 @@ jobs: mkdir -p ./build docker compose --project-directory ./systemd-build up --build ./generate-changelog > build/CHANGELOG - ./gradlew test buildPlugin + ./gradlew test buildPlugin -Dsystemd.unit.grammarParseEngine=${{ matrix.grammarParseEngine }} ./gradlew --stop - name: Publish Unit Test Results uses: EnricoMi/publish-unit-test-result-action@v2 if: always() with: + check_name: "Unit Test Results (grammar engine ${{ matrix.grammarParseEngine }})" junit_files: build/test-results/**/*.xml diff --git a/build.gradle.kts b/build.gradle.kts index b43f36cf..46ed4176 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -144,6 +144,10 @@ tasks { test { testLogging.showExceptions = true testLogging.setExceptionFormat("full") + // Forward the grammar-engine switch to the forked test JVM so the suite can be run twice: + // ./gradlew test (original validation engine) + // ./gradlew test -Dsystemd.unit.grammarParseEngine=true (new list-of-successes engine) + systemProperty("systemd.unit.grammarParseEngine", System.getProperty("systemd.unit.grammarParseEngine", "false")) } } diff --git a/src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/grammar/GrammarOptionValue.kt b/src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/grammar/GrammarOptionValue.kt index 33375edb..a8567bf9 100644 --- a/src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/grammar/GrammarOptionValue.kt +++ b/src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/grammar/GrammarOptionValue.kt @@ -37,7 +37,7 @@ open class GrammarOptionValue( override fun generateProblemDescriptors(property: UnitFilePropertyType, holder: ProblemsHolder) { val value = property.valueText ?: return - if (ExperimentalSettings.getInstance(property.project).state.useGrammarParseEngine) { + if (FORCE_PARSE_ENGINE || ExperimentalSettings.getInstance(property.project).state.useGrammarParseEngine) { generateProblemDescriptorsViaParse(property, value, holder) return } @@ -173,5 +173,15 @@ open class GrammarOptionValue( companion object { private val LOG = Logger.getInstance(SemanticDataRepository::class.java) + + /** + * Forces the new list-of-successes engine for validation regardless of the per-project setting, + * used to run the whole unit-test suite against it (CI runs the suite twice: once without and + * once with -Dsystemd.unit.grammarParseEngine=true). Only the validation engine is forced; the + * cosmetic annotators stay on the user flag, so problem counts are unchanged and only exact + * error spans/messages can differ between engines. + */ + @JvmField + val FORCE_PARSE_ENGINE: Boolean = java.lang.Boolean.getBoolean("systemd.unit.grammarParseEngine") } } diff --git a/src/test/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/inspections/InvalidValueInspectionForCGroupSocketBind.kt b/src/test/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/inspections/InvalidValueInspectionForCGroupSocketBind.kt index 0701bebb..042f5304 100644 --- a/src/test/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/inspections/InvalidValueInspectionForCGroupSocketBind.kt +++ b/src/test/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/inspections/InvalidValueInspectionForCGroupSocketBind.kt @@ -2,6 +2,7 @@ package net.sjrx.intellij.plugins.systemdunitfiles.inspections import junit.framework.TestCase import net.sjrx.intellij.plugins.systemdunitfiles.AbstractUnitFileTest +import net.sjrx.intellij.plugins.systemdunitfiles.semanticdata.optionvalues.grammar.GrammarOptionValue class InvalidValueInspectionForCGroupSocketBindOptionValue : AbstractUnitFileTest() { @@ -133,7 +134,9 @@ class InvalidValueInspectionForCGroupSocketBindOptionValue : AbstractUnitFileTes assertSize(1, highlights) val info = highlights[0] assertStringContains("SocketBindAllow's value does not match the expected format.", info!!.description) - TestCase.assertEquals("::tcp", info.text) + // The two engines localize this differently: the original highlights from after "ipv6"; the + // list-of-successes engine consumes "ipv6:" before getting stuck, so it highlights from there. + TestCase.assertEquals(if (GrammarOptionValue.FORCE_PARSE_ENGINE) ":tcp" else "::tcp", info.text) } fun testWeakWarningWhenInvalidPortRangeSpecified() { @@ -154,6 +157,8 @@ class InvalidValueInspectionForCGroupSocketBindOptionValue : AbstractUnitFileTes assertSize(1, highlights) val info = highlights[0] assertStringContains("SocketBindAllow's value is correctly formatted but seems invalid.", info!!.description) - TestCase.assertEquals("-", info.text) + // The original engine gives up at the first "-"; the list-of-successes engine parses the whole + // range form and points at the out-of-range port "-21485". + TestCase.assertEquals(if (GrammarOptionValue.FORCE_PARSE_ENGINE) "-21485" else "-", info.text) } }