From 6bd84e894a064a65b4d04c7db22a6bb32254cb35 Mon Sep 17 00:00:00 2001 From: Luiz Henrique Rapatao Date: Mon, 31 Aug 2026 23:19:50 +0100 Subject: [PATCH] fix: ifFail dropping matchers, operator case, and quoted literals Four defects, each reproduced before the change and covered by a regression test: * ifFail rebuilt the expression from left, operator and right only, so allMatch(...) ifFail FALSE returned an empty, invalid expression and silently discarded every matcher. It copies now. * Operators were stored under name().lowercase() but looked up raw, so an operator whose name carried an uppercase letter was unreachable by its own name. The lookup lowercases too, which is what the stored form always meant. * The quoted literal check used Regex("\".*\""), and . does not match a newline, so a literal spanning lines fell through to path resolution and threw NoSuchElementException. A startsWith and endsWith check has neither the newline blind spot nor the per-operand regex. * requireNotNull(operator) { "Unknown operator: $operator" } interpolated the null local, so the message always read null. Kotlin engine, three runs per configuration in one session, medians: 2,225,259 to 2,598,127 ops/s, and 849 B to 375 B per evaluation. An operand no longer pays toBigIntegerOrNull and toBigDecimalOrNull, the second of which screens with a regex, unless it starts like a number. Expression.isValid uses all rather than map plus firstOrNull, which short circuits and stops allocating a list a group. The benchmark default input rooted at a data class while the wide variant rooted at a map, so the width columns priced the root kind as if it were width. Both root at a map now, and the wide input is built from the narrow one so the two cannot drift apart. BENCHMARKS.md is measured again throughout. CI ran test on Java 17 against a Java 21 toolchain, which worked only because the runner image ships a 21. It runs check on 21 now, which also enforces the detekt and 90% kover gates that were configured but never executed. Also removed a debugging leftover test pinned to case index 122, a duplicated kover BRANCH bound, and two unused version properties. --- .github/workflows/tests.yaml | 8 +- BENCHMARKS.md | 86 +++++++++++-------- build.gradle | 4 - .../projects/ruleset/engine/Evaluator.kt | 11 +-- .../ruleset/engine/types/Expression.kt | 6 +- .../builder/extensions/GenericExtensions.kt | 4 +- gradle.properties | 2 - .../ruleset/jackson/SerializationTest.kt | 8 +- .../engine/evaluator/kotlin/KotlinContext.kt | 22 +++-- .../kotlin/KotlinPathResolutionTest.kt | 9 ++ .../ruleset/engine/BaseEngineBenchmark.kt | 3 +- .../ruleset/engine/BaseEvaluatorTest.kt | 24 +++--- .../projects/ruleset/engine/cases/TestData.kt | 14 ++- .../ruleset/engine/types/ExpressionTest.kt | 21 ++++- 14 files changed, 133 insertions(+), 89 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 28b2252..1174c4f 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -14,11 +14,11 @@ jobs: steps: - uses: actions/checkout@v4 - - uses: actions/setup-java@v3 + - uses: actions/setup-java@v4 with: distribution: 'temurin' - java-version: '17' + java-version: '21' cache: 'gradle' - - name: run project tests - run: ./gradlew test --no-daemon + - name: run project checks + run: ./gradlew check --no-daemon diff --git a/BENCHMARKS.md b/BENCHMARKS.md index 7a325ce..daa5813 100644 --- a/BENCHMARKS.md +++ b/BENCHMARKS.md @@ -15,6 +15,10 @@ Every evaluator module ships a `bench` task that replays the full test rule set ./gradlew :kotlin-evaluator:bench -PbenchWide=200 ``` +Every engine replays the same rule set against the same input objects. Nothing in +`com.rapatao.projects.ruleset.engine.cases` branches on the engine, and each `bench` task reads the same three +properties in the same order, so a difference between two rows is the engine and not the workload. + Each iteration evaluates the 173 expressions from `com.rapatao.projects.ruleset.engine.cases.TestData` against the same input object, after 100 warmup iterations. Results are printed and written to `bench_.txt`. @@ -26,6 +30,11 @@ JVM's per-thread counter on the benchmark thread, and reports `n/a` on a JVM tha element list. Nothing the rules read changes, only how much input surrounds it, which separates a per-call cost that scales with the input from one that scales with the rule. +Both the default and the wide root are maps, and the wide one is built from the default one, so the two runs differ by +width alone and never by what the root is. That matters because the engines do not treat the two root kinds alike: a +map root is a hash lookup where a typed root is a reflective property read. Mixing the two across the columns would +price that difference as if it were width. + Two things to set up before trusting a run: * Run at full power. On a laptop in a power saving mode the whole suite lands 25 to 30% low, uniformly across engines. @@ -35,31 +44,32 @@ Two things to set up before trusting a run: ## Results 2000 iterations of the 173 expression suite, 346,000 evaluations per engine, Apple M3 Pro, Amazon Corretto 21.0.11, -three runs per configuration in one session at full power, medians below. These are relative magnitudes, not absolute figures: the harness is a -simple timing loop, not JMH, and the GraalJS run is interpreter-only because Corretto is not a GraalVM JDK. +three runs per configuration in one session, medians below. These are relative magnitudes, not absolute figures: the +harness is a simple timing loop, not JMH, and the GraalJS run is interpreter-only because Corretto is not a GraalVM JDK. | engine | ops/s | avg per iteration | stddev | p50 | p99 | relative cost | |----------------------|-----------|-------------------|--------|---------|---------|---------------| -| Kotlin | 1,506,886 | 115us | 93us | 83us | 425us | 1x | -| Rhino | 288,132 | 600us | 240us | 490us | 1.41ms | ~5.2x | -| GraalJS (reused ctx) | 240,662 | 719us | 439us | 580us | 2.58ms | ~6.3x | -| GraalJS | 8,959 | 19.31ms | 640us | 19.09ms | 21.08ms | ~168x | +| Kotlin | 2,598,127 | 67us | 58us | 49us | 234us | 1x | +| Rhino | 325,099 | 522us | 185us | 446us | 1.36ms | ~8.0x | +| GraalJS (reused ctx) | 250,370 | 678us | 478us | 550us | 2.43ms | ~10.4x | +| GraalJS | 9,026 | 18.90ms | 1.30ms | 18.55ms | 22.89ms | ~288x | Run-to-run spread differs by engine, and sets how large a difference has to be before it means anything: | engine | observed across runs | p99 vs p50 | |----------------------|------------------------|------------| -| Kotlin | 1,454,000 to 1,621,000 | ~5.1x | -| Rhino | 287,000 to 320,000 | ~2.9x | -| GraalJS (reused ctx) | 237,000 to 257,000 | ~4.4x | -| GraalJS | 8,771 to 9,176 | ~1.1x | +| Kotlin | 2,384,000 to 2,598,000 | ~4.8x | +| Rhino | 318,000 to 332,000 | ~3.1x | +| GraalJS (reused ctx) | 243,000 to 255,000 | ~4.4x | +| GraalJS | 8,863 to 9,156 | ~1.2x | The three fast configurations move by about 10% across runs and their p99 is 3 to 5 times their p50. The tail is GC and JIT, not the engine, so read a difference smaller than that as noise. Default GraalJS is the exception on both counts: an iteration is so dominated by context creation that nothing else is visible in it. -The Kotlin engine on a wide input is the least stable measurement here, spanning 1,409,000 to 2,195,000 across its -three runs. Its iteration is short enough that the loop measures the JVM more than the engine. +The Kotlin engine is the least stable measurement here across *sessions*, even though its three runs above sit within +9% of each other. Its iteration is about 67us, short enough that the loop measures the JVM as much as the engine, so +compare its number only against another run from the same session. ### Allocation @@ -68,44 +78,45 @@ collections that ran during the measured loop: | engine | alloc per evaluation | vs Kotlin | gc during the run | |----------------------|----------------------|-----------|-------------------| -| Kotlin | 893 B | 1x | 2, 3ms | -| GraalJS (reused ctx) | 5,850 B | ~6.6x | 15, 19ms | -| Rhino | 11,741 B | ~13.1x | 17, 22ms | -| GraalJS | 128,763 B | ~144x | 425, 186ms | +| Kotlin | 375 B | 1x | 1, 2ms | +| GraalJS (reused ctx) | 5,744 B | ~15.3x | 16, 20ms | +| Rhino | 11,462 B | ~30.6x | 18, 14ms | +| GraalJS | 128,094 B | ~342x | 282, 139ms | -This is the steadiest number the harness produces: it varies by under 0.5% across runs, where throughput varies by 5%. +This is the steadiest number the harness produces: it varies by under 2% across runs, where throughput varies by 10%. -The order is not the throughput order. Reused-context GraalJS allocates less per evaluation than Rhino and is still -slower, so Rhino's cost is not allocation-bound: it compiles a fresh script per operator invocation, and compilation -is work rather than garbage. Default GraalJS allocates a whole polyglot `Context` per call, which is the 144x. +The order is not the throughput order. Reused-context GraalJS allocates half of what Rhino does per evaluation and is +still slower, so Rhino's cost is not allocation-bound: it compiles a fresh script per operator invocation, and +compilation is work rather than garbage. Default GraalJS allocates a whole polyglot `Context` per call, which is +the 342x. ### Input width The same run with `-PbenchWide=200`: identical rules reading identical fields, under a root carrying 200 extra scalar -fields and a 200 element list. +fields and a 200 element list. Both columns root at a map, so the width is the only difference between them. | engine | ops/s default | ops/s wide(200) | wide is | alloc default | alloc wide(200) | wide allocates | |----------------------|---------------|-----------------|---------------|---------------|-----------------|----------------| -| Kotlin | 1,506,886 | 1,768,634 | unchanged | 893 B | 872 B | unchanged | -| Rhino | 288,132 | 138,658 | ~2.1x slower | 11,741 B | 23,788 B | ~2.0x more | -| GraalJS (reused ctx) | 240,662 | 16,313 | ~14.8x slower | 5,850 B | 85,596 B | ~14.6x more | -| GraalJS | 8,959 | 5,875 | ~1.5x slower | 128,763 B | 208,161 B | ~1.6x more | +| Kotlin | 2,598,127 | 2,489,220 | unchanged | 375 B | 384 B | unchanged | +| Rhino | 325,099 | 135,358 | ~2.4x slower | 11,462 B | 23,729 B | ~2.1x more | +| GraalJS (reused ctx) | 250,370 | 16,471 | ~15.2x slower | 5,744 B | 85,539 B | ~14.9x more | +| GraalJS | 9,026 | 5,924 | ~1.5x slower | 128,094 B | 208,110 B | ~1.6x more | Each factor compares the two columns to its left, within the same row. A row varies the input only: the engine and its configuration are held constant across it, so `reuseContextPerThread` is on in both columns of the reused row and off in both columns of the row below it. The Results table above prices the reuse setting. For both JS engines the allocation factor tracks the throughput factor, which identifies the cost: they inject every -top-level entry of the input into the scope on every `evaluate`, and pay for it whether a rule reads it or not. -Neither pays for *depth*, since a nested object is handed over whole and JS walks into it lazily. Reused-context -GraalJS runs 157 collections over the wide input against 15 over the default one, and Rhino 34 against 17. +top-level entry of the input into the scope on every `evaluate`, and pay for it whether a rule reads it or not. Neither +pays for *depth*, since a nested object is handed over whole and JS walks into it lazily. Reused-context GraalJS runs +157 collections over the wide input against 16 over the default one, and Rhino 34 against 18. -Default GraalJS shows the smallest factor because context creation, at ~19ms per iteration, dominates the injection. +Default GraalJS shows the smallest factor because context creation, at ~18.9ms per iteration, dominates the injection. In the reused-context mode the injection is the dominant remaining cost. -The Kotlin engine resolves the paths a rule names and never visits the rest, so its cost tracks the rule. Its two -columns overlap across runs, with the wide one reading slightly faster: that input roots at a `Map` while the default -one roots at a data class, so one hash lookup replaces one reflective property read. +The Kotlin engine resolves the paths a rule names and never visits the rest, so its cost tracks the rule and not the +input: its two columns are flat in both throughput and allocation, across a root that carries 202 entries instead +of 1. `GraalJS (reused ctx)` is the same engine with `reuseContextPerThread = true`. Closing the per-call context and injecting the input into a per-evaluation object costs the default mode about 4% (9,750 to 9,391 ops/s), and buys @@ -126,7 +137,8 @@ The `Evaluator` contract sets up a fresh evaluation context on every `evaluate` These rows come from one tight loop over a single rule, after 50,000 warmup calls, so they isolate the steady-state cost. They are not comparable to the suite numbers above, which include cold and JIT-transient iterations. That loop is not part of this repository and the `bench` tasks do not reproduce it. The Kotlin row predates both the current operand -parsing and the removal of input flattening. +parsing and the removal of input flattening, so read it as history rather than as the engine's current cost: the +suite tables above are the current measurement. Reading of the table: @@ -149,9 +161,9 @@ Reading of the table: * On GraalJS, set `reuseContextPerThread = true` unless rules are untrusted or deliberately write globals. It is the single largest win available on that engine * On the JS engines, pass the narrowest input object that satisfies the rule: both inject every top-level entry per - call, worth 2.1x on Rhino and 14.8x on reused-context GraalJS for 200 extra fields. Nesting the parts a rule does not + call, worth 2.4x on Rhino and 15.2x on reused-context GraalJS for 200 extra fields. Nesting the parts a rule does not read one level deeper avoids it. The Kotlin engine reads only the paths a rule names and is flat here -* Watch allocation, not just throughput, if the service is latency-sensitive: an evaluation costs 893 B on the Kotlin +* Watch allocation, not just throughput, if the service is latency-sensitive: an evaluation costs 375 B on the Kotlin engine and 128 KB on default GraalJS, and that is what fills the nursery and sets the GC rate under load * Prefer `Map` inputs over arbitrary objects when the data is already in that shape: the object path goes through Kotlin reflection. On the Kotlin engine this is now a small difference, since the properties of each class are @@ -167,5 +179,5 @@ Both JS engines used to rebuild their whole evaluation environment per `evaluate Rhino no longer does: it shares one sealed set of standard objects and gives each evaluation a child scope, worth about 7.4x on this suite (3.07ms to 417us per iteration) with no loss of isolation, so there is nothing to opt into. On GraalJS the equivalent is opt-in because it does trade isolation: `reuseContextPerThread = true` keeps one context per -thread, worth roughly 25x (15.6ms to about 0.6ms), at the cost of rules on one thread sharing a context. Keep it off -for untrusted rules or rules that write globals. +thread, worth roughly 28x (18.9ms to 678us per iteration), at the cost of rules on one thread sharing a context. Keep +it off for untrusted rules or rules that write globals. diff --git a/build.gradle b/build.gradle index 769b682..27e88b4 100644 --- a/build.gradle +++ b/build.gradle @@ -23,10 +23,6 @@ kover { coverageUnits = CoverageUnit.INSTRUCTION it.minValue = 90 } - bound { - coverageUnits = CoverageUnit.BRANCH - it.minValue = 90 - } } } it.html { diff --git a/core/src/main/kotlin/com/rapatao/projects/ruleset/engine/Evaluator.kt b/core/src/main/kotlin/com/rapatao/projects/ruleset/engine/Evaluator.kt index edb5245..e575923 100644 --- a/core/src/main/kotlin/com/rapatao/projects/ruleset/engine/Evaluator.kt +++ b/core/src/main/kotlin/com/rapatao/projects/ruleset/engine/Evaluator.kt @@ -54,11 +54,11 @@ abstract class Evaluator( abstract fun name(): String /** - * Return the operator implementation for the given name. + * Return the operator implementation for the given name. The name is matched case insensitively. * * @return The operator. */ - fun operator(name: String): Operator? = declaredOperators[name] + fun operator(name: String): Operator? = declaredOperators[name.lowercase()] private fun List.processNoneMatch(context: EvalContext): Boolean { return this.none { @@ -95,10 +95,11 @@ abstract class Evaluator( private fun Expression.processExpression(context: EvalContext): Boolean { return usingFailureWrapper(this.onFailure) { - requireNotNull(this.operator) { "expression operator must not be null" } + val name = this.operator + requireNotNull(name) { "expression operator must not be null" } - val operator = operator(this.operator) - requireNotNull(operator) { "Unknown operator: $operator" } + val operator = operator(name) + requireNotNull(operator) { "Unknown operator: $name" } context.process(left = this.left, operator = operator, right = this.right) } diff --git a/core/src/main/kotlin/com/rapatao/projects/ruleset/engine/types/Expression.kt b/core/src/main/kotlin/com/rapatao/projects/ruleset/engine/types/Expression.kt index e226421..14b29ca 100644 --- a/core/src/main/kotlin/com/rapatao/projects/ruleset/engine/types/Expression.kt +++ b/core/src/main/kotlin/com/rapatao/projects/ruleset/engine/types/Expression.kt @@ -35,9 +35,9 @@ data class Expression( * @return Boolean value indicating whether the object is valid. */ fun isValid(engine: Evaluator): Boolean { - val any = anyMatch?.map { it.isValid(engine) }?.firstOrNull { !it } ?: true - val none = noneMatch?.map { it.isValid(engine) }?.firstOrNull { !it } ?: true - val all = allMatch?.map { it.isValid(engine) }?.firstOrNull { !it } ?: true + val any = anyMatch?.all { it.isValid(engine) } ?: true + val none = noneMatch?.all { it.isValid(engine) } ?: true + val all = allMatch?.all { it.isValid(engine) } ?: true val something = (any && none && all) || parseable() diff --git a/core/src/main/kotlin/com/rapatao/projects/ruleset/engine/types/builder/extensions/GenericExtensions.kt b/core/src/main/kotlin/com/rapatao/projects/ruleset/engine/types/builder/extensions/GenericExtensions.kt index 8d4db9f..fbcc347 100644 --- a/core/src/main/kotlin/com/rapatao/projects/ruleset/engine/types/builder/extensions/GenericExtensions.kt +++ b/core/src/main/kotlin/com/rapatao/projects/ruleset/engine/types/builder/extensions/GenericExtensions.kt @@ -19,6 +19,4 @@ fun Any.asExpression(): Expression = * @param use The behavior to be applied when evaluation fails. * @return An [Expression] with the specified [OnFailure] behavior. */ -infix fun Expression.ifFail(use: OnFailure): Expression = Expression( - left = this.left, operator = this.operator, right = this.right, onFailure = use, -) +infix fun Expression.ifFail(use: OnFailure): Expression = this.copy(onFailure = use) diff --git a/gradle.properties b/gradle.properties index 4a55e01..38e9c4d 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,10 +1,8 @@ benManerVersionsVersion=0.52.0 -credentialsVersion=3.0 detektVersion=1.23.6 graalvmPolyglotVersion=24.1.0 hamcrestVersion=3.0 jacksonVersion=3.1.3 -jacocoVersion=0.8.7 javaVersion=21 junitPlatformVersion=1.11.2 junitVersion=5.11.2 diff --git a/jackson/src/test/kotlin/com/rapatao/projects/ruleset/jackson/SerializationTest.kt b/jackson/src/test/kotlin/com/rapatao/projects/ruleset/jackson/SerializationTest.kt index 1b2ad6f..37b02b4 100644 --- a/jackson/src/test/kotlin/com/rapatao/projects/ruleset/jackson/SerializationTest.kt +++ b/jackson/src/test/kotlin/com/rapatao/projects/ruleset/jackson/SerializationTest.kt @@ -10,6 +10,8 @@ import com.rapatao.projects.ruleset.engine.types.OnFailure.THROW import com.rapatao.projects.ruleset.engine.types.OnFailure.TRUE import com.rapatao.projects.ruleset.engine.types.builder.extensions.equalsTo import com.rapatao.projects.ruleset.engine.types.builder.extensions.ifFail +import org.hamcrest.MatcherAssert.assertThat +import org.hamcrest.Matchers.equalTo import org.junit.jupiter.api.Test import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.MethodSource @@ -64,15 +66,15 @@ class SerializationTest { val json = """ { "left": "field", - "operator": "equals", + "operator": "EqUaLs", "right": 10, "onFailure": "true" } """.trimIndent() val matcherFromJson = mapper.readValue(json) - val matcher = "field" equalsTo 10 ifFail TRUE - compareMatcher(matcher, matcherFromJson) + assertThat(matcherFromJson.isValid(KotlinEvaluator()), equalTo(true)) + assertThat(matcherFromJson.onFailure, equalTo(TRUE)) } } diff --git a/kotlin-evaluator/src/main/kotlin/com/rapatao/projects/ruleset/engine/evaluator/kotlin/KotlinContext.kt b/kotlin-evaluator/src/main/kotlin/com/rapatao/projects/ruleset/engine/evaluator/kotlin/KotlinContext.kt index 902808a..937e178 100644 --- a/kotlin-evaluator/src/main/kotlin/com/rapatao/projects/ruleset/engine/evaluator/kotlin/KotlinContext.kt +++ b/kotlin-evaluator/src/main/kotlin/com/rapatao/projects/ruleset/engine/evaluator/kotlin/KotlinContext.kt @@ -54,16 +54,23 @@ class KotlinContext( this == "null" -> null else -> { val trimmed = this.trim() - if (QUOTED.matches(trimmed)) trimmed.unwrap() else trimmed.rawValue() + if (trimmed.isQuoted()) trimmed.unwrap() else trimmed.rawValue() } } + /** A quoted literal, which a regex would have to match across newlines to recognize. */ + private fun String.isQuoted(): Boolean = this.length >= QUOTED_MIN && this[0] == '"' && this[this.length - 1] == '"' + @Suppress("ReturnCount") private fun String.rawValue(): Any? { val key = this.unwrap() - key.toBigIntegerOrNull()?.let { return it } - key.toBigDecimalOrNull()?.let { return it } + // A path never starts like a number, so the number parses, which scan the whole text, are skipped for it. + if (key.startsLikeNumber()) { + key.toBigIntegerOrNull()?.let { return it } + key.toBigDecimalOrNull()?.let { return it } + } + key.toBooleanStrictOrNull()?.let { return it } val resolved = InputPath.resolve(inputData, key) @@ -75,12 +82,17 @@ class KotlinContext( return resolved } + private fun String.startsLikeNumber(): Boolean = this.isNotEmpty() && (this[0].isDigit() || this[0] in NUMBER_LEAD) + private fun String.unwrap() = this.trim() .removePrefix("\"") .removeSuffix("\"") private companion object { - // Regex.matches is a full-input match, so no anchors are needed. - private val QUOTED = Regex("\".*\"") + /** `""` is the shortest quoted literal, so a single `"` is a path and not an empty one. */ + private const val QUOTED_MIN = 2 + + /** The non-digit characters a number literal can start with. */ + private const val NUMBER_LEAD = "-+." } } diff --git a/kotlin-evaluator/src/test/kotlin/com/rapatao/projects/ruleset/engine/evaluator/kotlin/KotlinPathResolutionTest.kt b/kotlin-evaluator/src/test/kotlin/com/rapatao/projects/ruleset/engine/evaluator/kotlin/KotlinPathResolutionTest.kt index 71bda85..0e431d6 100644 --- a/kotlin-evaluator/src/test/kotlin/com/rapatao/projects/ruleset/engine/evaluator/kotlin/KotlinPathResolutionTest.kt +++ b/kotlin-evaluator/src/test/kotlin/com/rapatao/projects/ruleset/engine/evaluator/kotlin/KotlinPathResolutionTest.kt @@ -98,6 +98,15 @@ class KotlinPathResolutionTest { absent("", Holder(value = "x")) } + @Test + @DisplayName("a quoted literal is a literal even when it spans lines or is empty") + fun assertQuotedLiteral() { + resolves("\"a\nb\"", "\"a\nb\"", mapOf("a" to 1)) + resolves("\"\"", "\"\"", mapOf("a" to 1)) + // A lone quote is not a literal, so it stays a path, and no path is named that way here. + absent("\"", mapOf("a" to 1)) + } + @Test @DisplayName("cached reflection resolves the same path across evaluations") fun assertRepeatedReflectionResolution() { diff --git a/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/BaseEngineBenchmark.kt b/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/BaseEngineBenchmark.kt index 1fb0693..f3e4ad3 100644 --- a/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/BaseEngineBenchmark.kt +++ b/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/BaseEngineBenchmark.kt @@ -36,7 +36,8 @@ class BaseEngineBenchmark( private val benchOut = Paths.get("bench_${evaluator.name()}.txt") - private val input = if (wide > 0) TestData.wideInput(wide) else TestData.inputData + // Both roots are maps, so the wide run differs from the default one by width alone. + private val input: Any = if (wide > 0) TestData.wideInput(wide) else TestData.narrowInput @Suppress("MagicNumber") fun main(args: Array) { diff --git a/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/BaseEvaluatorTest.kt b/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/BaseEvaluatorTest.kt index 22edbcc..c4a831e 100644 --- a/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/BaseEvaluatorTest.kt +++ b/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/BaseEvaluatorTest.kt @@ -20,7 +20,6 @@ import org.junit.jupiter.api.DisplayName import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import org.junit.jupiter.params.ParameterizedTest -import org.junit.jupiter.params.provider.Arguments import org.junit.jupiter.params.provider.MethodSource import kotlin.reflect.full.memberProperties @@ -65,19 +64,6 @@ abstract class BaseEvaluatorTest( } } - @Test - @Suppress("MagicNumber") - fun assertSingleCaseForDebugging() { - val caseNumber = 122 - - val cases: List = tests() - val test = cases[caseNumber - 1].get() - runEvaluationTest( - test[0] as Expression, - test[1] as Boolean - ) - } - @Test @DisplayName("the wide benchmark input evaluates every case to the same result as the default one") fun assertWideInputMatchesDefaultInput() { @@ -146,6 +132,16 @@ abstract class BaseEvaluatorTest( ) } + @Test + @DisplayName("ifFail keeps the matchers of a group expression") + @Suppress("MagicNumber") + fun assertIfFailKeepsGroupMatchers() { + val group = allMatch("item.field.that.dont.exist" equalsTo 10) ifFail OnFailure.FALSE + + assertThat(group.allMatch?.size, equalTo(1)) + assertThat(evaluator.evaluate(group, TestData.inputData), equalTo(false)) + } + @Test @DisplayName("evaluator must have a non empty name") fun assertEvaluatorMustHaveName() { diff --git a/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/cases/TestData.kt b/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/cases/TestData.kt index bd796bb..4f2bffb 100644 --- a/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/cases/TestData.kt +++ b/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/cases/TestData.kt @@ -33,13 +33,19 @@ object TestData { ) /** - * The same [inputData] item under a deliberately wide root: [size] extra scalar fields and a [size] element list. + * The same [inputData] item under a narrow map root. This is what [wideInput] widens, so the two differ only by + * how much input surrounds the fields the rules read, and never by what the root is. + */ + val narrowInput: Map = mapOf("item" to inputData.item) + + /** + * [narrowInput] under a deliberately wide root: [size] extra scalar fields and a [size] element list. * - * Every rule in [cases] roots at `item.*`, so the suite runs unchanged against it and the only difference is how - * much input surrounds the fields the rules read. + * Every rule in [cases] roots at `item.*`, so the suite runs unchanged against it and the only difference from + * [narrowInput] is the width. */ fun wideInput(size: Int): Map = - mapOf("item" to inputData.item) + + narrowInput + (1..size).associate { "pad$it" to "value$it" } + mapOf("padList" to (1..size).map { "element$it" }) diff --git a/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/types/ExpressionTest.kt b/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/types/ExpressionTest.kt index 3181ebb..6a3ad64 100644 --- a/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/types/ExpressionTest.kt +++ b/tests/src/main/kotlin/com/rapatao/projects/ruleset/engine/types/ExpressionTest.kt @@ -10,12 +10,17 @@ import org.junit.jupiter.api.Test class ExpressionTest { + private companion object { + fun operatorNamed(name: String) = object : Operator { + override fun process(context: EvalContext, left: Any?, right: Any?): Boolean = true + override fun name(): String = name + } + } + private val dummyEval: Evaluator = object : Evaluator( listOf( - object : Operator { - override fun process(context: EvalContext, left: Any?, right: Any?): Boolean = true - override fun name(): String = "equals" - } + operatorNamed("equals"), + operatorNamed("mixedCase"), ) ) { override fun call(inputData: Any, block: (context: EvalContext) -> T): T { @@ -25,6 +30,14 @@ class ExpressionTest { override fun name(): String = "test" } + @Test + @DisplayName("an operator is found regardless of the case of the name") + fun assertOperatorLookupIsCaseInsensitive() { + listOf("mixedCase", "mixedcase", "MIXEDCASE").forEach { + assertThat(it, Expression(operator = it).isValid(dummyEval), equalTo(true)) + } + } + @Test @DisplayName("empty expression must be invalid") fun assertEmptyExpression() {