Conversation
… calculator Twenty-four defects in MongoHeuristicsCalculator, each as its own @disabled test, so they can be enabled one at a time and in any order as the behaviour is implemented. Every expected value was obtained by running the same query and the same document against a real MongoDB 7.0.40 server, so the assertions state what the database does rather than an interpretation of the documentation. Ten of them make the calculator throw. As MongoHandler does not catch anything, the exception escapes the heuristics computation for the action, so the ExtraHeuristicsDto is lost, including the SQL heuristics computed before it: $type with a string alias or with a list of aliases, a bitmask given as an Integer or as a list of bit positions, and $not holding more than one operator, are not parsed, and the calculator throws a NullPointerException on the resulting null operation; an operator that is not modelled at all is not parsed either, which covers $expr, $jsonSchema, $where, $text, $geoWithin, $geoIntersects and $comment. The last one attaches to an otherwise ordinary query, so {"a": 1, "$comment": "..."} is enough to lose the heuristics of the action; an empty list of values, an empty array in the document, and comparing two empty arrays aggregate over no element and throw IllegalArgumentException; an ordering comparison involving NaN builds a Truthness with neither of its values equal to 1, which its own constructor rejects. Any double field can hold NaN; a value the calculator cannot compare, such as a sub-document or binary data, reaches the "Unsupported type" branch and throws. The test for the operators that are not modelled asserts only that nothing is thrown, not any particular score: whether such an operator should be supported, and what it should answer, is a decision for the heuristic. What it should not do is cost the action its heuristics. The other fourteen are answered, but not the way MongoDB answers them. Those that report a match where MongoDB has none are the harmful direction, as a condition no data can satisfy is recorded as covered and the search stops working towards it. Several share one rule: MongoDB matches a field when its value satisfies the condition, or when it holds an array of which any element does, and that applies to every condition on a field rather than only to equality. $all is the same rule quantified the other way round, over the expected values. Four tests of behaviour that is already correct are added as well, as a guard while the heuristic is changed. One of them, testNotEqualsAgainstAnArrayField, answers correctly only because two of the reported defects cancel each other out; fixing either one alone turns it into a false positive, which is why it is worth keeping visible.
…ses; re-enable disabled tests
…dge cases; add related test cases.
… bitmask operations
… re-enable disabled tests and migrate geospatial logic to `MongoUtils`.
…; adjust comparison logic for `NOT_EQUALS_TO` operator.
…ated `@Disabled` annotation.
…nd simplify logic
…gic to handle `$comment` operators
… handling of query operations
… logic, and improve modularity for comparison and bitwise operations
… logic, and improve modularity for comparison and bitwise operations
…ity and consistency.
There was a problem hiding this comment.
The answers quoted in the comments below are what a mongo 7.0.41 returns for those queries, not what the documentation implies.
Two queries mongo accepts make the calculator throw: $regex on an array holding no strings, and a BinData bitmask. The first one is a regression, the code this replaces returned C_FALSE for it. Nothing catches either on the way out of computeDistanceDocuments, so the action loses its SQL heuristics along with the mongo ones.
The rest are wrong answers rather than crashes. No validation on the bitmask itself, $comment stripped out of things that aren't operators, $all wrong on a repeated element.
Two that look wrong and aren't: nested $not agrees with mongo both ways, and the reversed actual/expected in evaluateListEquality does hit the taint handler backwards, but ExecutionTracer.handleTaintForStringEquals takes either direction so nothing comes of it.
Not your change, but the unparsed-query-then-NPE path that testOperatorsThatAreNotModelledDoNotThrow documents is bigger than that disabled test implies. {a:{$not:/x/}} lands in it, and that one mongo answers fine.
|
hi @jgaleotti @LautaroPetaccio I m confused here... @jgaleotti you asked my review, but @LautaroPetaccio did it first? in that case, if you asked @LautaroPetaccio to do it, you should then wait to fix his comments before asking my review. @LautaroPetaccio did you do such review? or was AI generated? there is plenty of sentences like |
|
Hi @arcuri82, I proactively reviewed the PR to help finding issues on the changes, @jgaleotti did not ask me to do it. I'm sorry if I caused any problems while doing so. The idea was to avoid introducing bugs to the MongoDB heuristic which would imply another re-review. The review was done with AI and fully tested against tests examples and MongoDB (which is the server here). I read the comments and they seemed ok, but your reply made me realize that the wording "the server" is quite ambiguous, which, although it states that the PR is working with MongoDB, in the context of EvoMaster, the wording is not enough. When talking about |
|
hi @LautaroPetaccio. |
…cs for $gte and $gt when both values are null
…er" alias type name.
|
@arcuri82: you might review this PR now. |
|
|
||
| static String getBsonTypeEnumNameFromAlias(String alias) { | ||
|
|
||
| switch (alias) { |
There was a problem hiding this comment.
i don't understand this switch. why not just returning alias.toUpperCase()?
| public static final double C = 0.1; | ||
| public static final Truthness C_FALSE = new Truthness(C, 1.0); | ||
| // TODO These constants should be refactored by TruthnessUtils constants | ||
| public static final Truthness TRUE_C = new Truthness(1.0, C); |
There was a problem hiding this comment.
why TODOs and not just doing it?
| this.taintHandler = taintHandler; | ||
| } | ||
|
|
||
| static Truthness buildSafeScaledTruthness(double maxOfTrue) { |
There was a problem hiding this comment.
why are these methods package-level scoped?
| return actualValue ? 1 : 0; | ||
| } | ||
|
|
||
| static Truthness buildSafeScaledTruthness(Truthness truthness) { |
There was a problem hiding this comment.
not going to repeat comment on package-level scoped, it applies to all method here
3 participants
Correct MongoDB query heuristics for arrays, null and missing fields, numeric comparisons, and bitwise operations. Extract shared evaluation logic into MongoHeuristicsCalculatorHelper.