Migrate base RulesEngineInternal operator unit tests to JSON predicate fixtures#3545
Conversation
…e fixtures Adds per-operator JSON predicate fixtures (exact copies of the iOS #6885 corpus) under src/test/resources/predicate-fixtures, run by a shared parameterized runner. Deletes the migrated arithmetic/comparison/equality/ logic suites and trims the accessor/evaluator/string-array suites to the cases not expressible as predicate->boolean. Test-only change. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3545 +/- ##
=======================================
Coverage 80.11% 80.11%
=======================================
Files 371 371
Lines 15166 15166
Branches 2100 2100
=======================================
Hits 12150 12150
Misses 2166 2166
Partials 850 850 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Expresses the "empty-string key resolves to the whole scope and is not
missing" case as a `{"!!":{"missing":[""]}}` -> false fixture (the string
coercion the other missing fixtures use can't distinguish [] from [""]).
Removes the now-covered Kotlin test and bumps the fixture count to 237.
Co-authored-by: Cursor <cursoragent@cursor.com>
Adds fixtures for the empty-segment dot-path splits, var default-vs-null-leaf behavior, and the non-numeric missing_some threshold (7 new cases), and drops the now-redundant Kotlin tests. AccessorOperatorsTest keeps only the cases that cannot be a predicate->boolean fixture (top-level array scope or a whole-object return). Fixture count is now 244. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
| expected.contains.forEach { substring -> | ||
| assertThat(warnings) | ||
| .withFailMessage("Fixture %s: missing warning containing \"%s\", got %s", id, substring, warnings) | ||
| .anyMatch { it.contains(substring) } |
There was a problem hiding this comment.
Could it be that we're not failing if we have the expected result + other results? In other words, is it exhaustive?
There was a problem hiding this comment.
That's right. My first iteration of this was indeed to make it exhaustive. However, I changed it intentionally. My reasoning was that if a specific test is checking that a specific warning is logged, then other logs that are independent from that test could be added/removed and that shouldn't affect that test passing.
There was a problem hiding this comment.
Alright, that also makes sense. Thanks!
| return (0 until fixtures.length()).map { parseCase(fixtures.getJSONObject(it)) } | ||
| } | ||
|
|
||
| private fun parseCase(json: JSONObject): PredicateConformanceFixtureCase = |
There was a problem hiding this comment.
I guess we could make PredicateConformanceFixtureCase serializable and use that... But TBH, I don't really mind. Happy to keep as is :)
There was a problem hiding this comment.
Right. I was doubting about this but Cursor told me there wasn't really a lot of benefit. Your comment was the last push I needed, so I migrated it in 53b9e92
I also like it better like this, being more similar to iOS
Co-authored-by: Cursor <cursoragent@cursor.com>
Android counterpart to RevenueCat/purchases-ios#6885
Motivation
Migrate the hand-written
RulesEngineInternaloperator unit tests into declarative JSON predicate fixtures run by one parameterized runner.Description
src/test/resources/predicate-fixtures, run by a shared parameterized runner. A guard test pins the loaded count to make sure we're not missing any test case.Made with Cursor
Note
Low Risk
Test-only refactor with no production code changes; risk is limited to possible coverage gaps if fixtures diverge from deleted Kotlin tests.
Overview
Moves most RulesEngineInternal operator and evaluator coverage from large Kotlin unit test classes into declarative JSON fixtures under
src/test/resources/predicate-fixtures/, aligned with the iOS PR.A parameterized
PredicateFixtureTestloads every fixture viaPredicateConformanceFixtureLoader(kotlinx.serialization) and asserts outcomes throughPredicateConformanceRunner—expected truthiness,RuleErrortypes, and optional warning substrings.PredicateFixtureLoaderTestpins 244 loaded cases so a broken resource load cannot shrink the suite silently.Removed standalone suites for arithmetic, comparison, equality, and logic; slimmed
EvaluatorTest,AccessorOperatorsTest, andStringArrayOperatorsTestto cases that cannot be expressed as predicate→boolean fixtures (malformed JSON parse, array-scopedvar, structuralmergenesting, whole-datavarresults).Test-only Gradle change: Kotlin serialization plugin plus
kotlinx.serialization.jsonon the test classpath. No production rules engine code changes.Reviewed by Cursor Bugbot for commit 53b9e92. Bugbot is set up for automated code reviews on this repo. Configure here.