yurloc commented on code in PR #5884:
URL: 
https://github.com/apache/incubator-kie-drools/pull/5884#discussion_r1590737801


##########
drools-drl/drools-drl-parser-tests/src/test/java/org/drools/drl/parser/antlr4/MiscDRLParserTest.java:
##########
@@ -4090,6 +4092,28 @@ void ooPathMixedWithStandardConstraint() {
                 .isEqualToIgnoringWhitespace("/wife[$age : age] && age > 
$age");
     }
 
+    @Test
+    void ooPathLhsPattern() {
+        final String text = "package org.drools\n"
+                + "rule PlainNot when\n"
+                + "    not( /strings [ this == \"It Does Work\" ] )\n"
+                + "then\n"
+                + "end\n";
+        PackageDescr packageDescr = parseAndGetPackageDescr(text);
+        RuleDescr ruleDescr = packageDescr.getRules().get(0);
+        
assertThat(ruleDescr.getLhs().getDescrs().get(0)).isInstanceOfSatisfying(NotDescr.class,
 notDescr -> {

Review Comment:
   > 1. what will be the printed message in case of failure in some of those 
nested assertions ?
   
   The nested assertions work just like regular top-level assertions. There is 
no difference in the failure message.
   
   The benefit of this nesting is that the lambda parameter (`notDescr ->`) 
type is the same class I'm using to do the instanceof check. So it saves me 
from casting the checked expression and declaring a new variable with the 
narrowed type if I want to do more assertions on it that require the specific 
type. The test code _should_ be smaller and easier to read.
   
   This is an equivalent code without nesting:
   ```java
   
           PackageDescr packageDescr = parseAndGetPackageDescr(text);
           RuleDescr ruleDescr = packageDescr.getRules().get(0);
           NotDescr notDescr = (NotDescr) ruleDescr.getLhs().getDescrs().get(0);
           assertThat(notDescr.getDescrs()).hasSize(1);
           PatternDescr patternDescr = (PatternDescr) 
notDescr.getDescrs().get(0);
           assertThat(patternDescr.getConstraint().getDescrs()).hasSize(1);
           ExprConstraintDescr exprConstraintDescr = (ExprConstraintDescr) 
patternDescr.getConstraint().getDescrs().get(0);
           assertThat(exprConstraintDescr.getExpression()).isEqualTo("/strings 
[ this == \"It Does Work\" ]");
           
assertThat(exprConstraintDescr.getType()).isEqualTo(ExprConstraintDescr.Type.NAMED);
           assertThat(exprConstraintDescr.getPosition()).isEqualTo(0);
   ```
   Note that I've left out `assertThat(...).isInstanceOf(Some.class);` which 
are kind of optional. They would simply give a nice failure message instead of 
a CCE. The same could be said about the `hasSize()` assertions.
   
   **TLDR;** Nested assertions' messages are identical. Using 
`.isInstanceOfSatisfying()` is a syntactic sugar. Probably just a matter of 
taste, and can be easily avoided.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to