Fixed rules overriding and added more informative error message in case there was error inside rule

Author: ishubin

galen-core/src/main/java/com/galenframework/speclang2/pagespec/PageSectionProcessor.java

@@ -30,10 +30,7 @@
 import org.apache.commons.lang3.tuple.Pair;
 
 import java.io.IOException;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 import java.util.regex.Matcher;
 
 public class PageSectionProcessor {
@@ -94,21 +91,21 @@ private void processSectionRule(PageSection section, StructNode ruleNode) throws
         List<StructNode> resultingNodes;
         try {
             resultingNodes = rule.getKey().apply(pageSpecHandler, ruleText, NO_OBJECT_NAME, rule.getValue(), ruleNode.getChildNodes());
+            processSection(ruleSection, resultingNodes);
         } catch (Exception ex) {
-            throw new SyntaxException(ruleNode, "Error processing custom rule", ex);
+            throw new SyntaxException(ruleNode, "Error processing rule: " + ruleText, ex);
         }
-        processSection(ruleSection, resultingNodes);
     }
 
     private Pair<PageRule, Map<String, String>> findAndProcessRule(String ruleText, StructNode ruleNode) {
-        List<Pair<Rule, PageRule>> rulePairs = pageSpecHandler.getPageRules();
+        ListIterator<Pair<Rule, PageRule>> iterator = pageSpecHandler.getPageRules().listIterator(pageSpecHandler.getPageRules().size());
         /*
-        It is important to reverse the rules list so that
+        It is important to make a reversed iteration over all rules so that
         it is possible for the end user to override previously defined rules
          */
-        Collections.reverse(rulePairs);
 
-        for (Pair<Rule, PageRule> rulePair : rulePairs) {
+        while (iterator.hasPrevious()) {
+            Pair<Rule, PageRule> rulePair = iterator.previous();
             Matcher matcher = rulePair.getKey().getPattern().matcher(ruleText);
             if (matcher.matches()) {
                 int index = 1;
@@ -133,17 +130,21 @@ private void processObjectLevelRule(ObjectSpecs objectSpecs, StructNode sourceNo
         String ruleText = sourceNode.getName().substring(1).trim();
         Pair<PageRule, Map<String, String>> rule = findAndProcessRule(ruleText, sourceNode);
 
-        pageSpecHandler.setGlobalVariable("objectName", objectSpecs.getObjectName(), sourceNode);
+        try {
+            pageSpecHandler.setGlobalVariable("objectName", objectSpecs.getObjectName(), sourceNode);
 
-        List<StructNode> specNodes = rule.getKey().apply(pageSpecHandler, ruleText, objectSpecs.getObjectName(), rule.getValue(), sourceNode.getChildNodes());
+            List<StructNode> specNodes = rule.getKey().apply(pageSpecHandler, ruleText, objectSpecs.getObjectName(), rule.getValue(), sourceNode.getChildNodes());
 
 
-        SpecGroup specGroup = new SpecGroup();
-        specGroup.setName(ruleText);
-        objectSpecs.addSpecGroup(specGroup);
+            SpecGroup specGroup = new SpecGroup();
+            specGroup.setName(ruleText);
+            objectSpecs.addSpecGroup(specGroup);
 
-        for (StructNode specNode : specNodes) {
-            specGroup.addSpec(pageSpecHandler.getSpecReader().read(specNode.getName(), pageSpecHandler.getContextPath()));
+            for (StructNode specNode : specNodes) {
+                specGroup.addSpec(pageSpecHandler.getSpecReader().read(specNode.getName(), pageSpecHandler.getContextPath()));
+            }
+        } catch (Exception ex) {
+            throw new SyntaxException(sourceNode, "Error processing rule: " + ruleText, ex);
         }
     }
 

galen-core/src/test/java/com/galenframework/tests/speclang2/pagespec/PageSpecReaderTest.java

@@ -909,7 +909,13 @@ public void ruleShouldHave_priorityOverRules_thatWereDeclaredFirst() throws IOEx
         PageSpec pageSpec = readPageSpec("speclang2/rule-priority.gspec");
 
         assertThat(pageSpec.getSections().size(), is(1));
-        assertThat(pageSpec.getSections().get(0).getObjects().size(), is(0));
+        assertThat(pageSpec.getSections().get(0).getObjects().size(), is(1));
+        assertThat(pageSpec.getSections().get(0).getObjects().get(0).getObjectName(), is("cancel_button"));
+        assertThat(pageSpec.getSections().get(0).getObjects().get(0).getSpecGroups().size(), is(1));
+        assertThat(pageSpec.getSections().get(0).getObjects().get(0).getSpecGroups().get(0).getSpecs().size(), is(1));
+        assertThat(pageSpec.getSections().get(0).getObjects().get(0).getSpecGroups().get(0).getSpecs().get(0).getOriginalText(), is("width 30px"));
+
+
         assertThat(pageSpec.getSections().get(0).getSections().size(), is(1));
         assertThat(pageSpec.getSections().get(0).getSections().get(0).getObjects().size(), is(1));
 
@@ -919,6 +925,18 @@ public void ruleShouldHave_priorityOverRules_thatWereDeclaredFirst() throws IOEx
         assertThat(object.getSpecs().get(0).getOriginalText(), is("width 300px"));
     }
 
+    @Test(expectedExceptions = FileSyntaxException.class,
+            expectedExceptionsMessageRegExp = "\\QError processing rule: button is located at the left side inside main_container with 10px margin\\E\\s+\\Qin speclang2/rule-error.gspec:7\\E")
+    public void shouldThrownInformativeError_whenThereIsProblemParsingTheRule() throws  IOException {
+            readPageSpec("speclang2/rule-error.gspec");
+    }
+
+    @Test(expectedExceptions = FileSyntaxException.class,
+            expectedExceptionsMessageRegExp = "\\QError processing rule: is located at the left side inside main_container with 10px margin\\E\\s+\\Qin speclang2/rule-error-object-level.gspec:7\\E")
+    public void shouldThrownInformativeError_whenThereIsProblemParsingTheRule_inObjectLevel() throws  IOException {
+        readPageSpec("speclang2/rule-error-object-level.gspec");
+    }
+
     @Test(expectedExceptions = FileSyntaxException.class,
             expectedExceptionsMessageRegExp = "\\QSpecs cannot have inner blocks\\E\\s+\\Qin speclang2/incorrect/nested-spec.gspec:7\\E")
     public void shouldGiveError_whenSpecIsNested_belowAnotherSpec() throws IOException {

galen-core/src/test/resources/speclang2/rule-error-object-level.gspec

@@ -0,0 +1,7 @@
+
+@rule is located at the %{side} side inside %{parentObject}
+    inside ${parentObject} 0px ${side}
+
+= Main =
+    button:
+        |is located at the left side inside main_container with 10px margin

galen-core/src/test/resources/speclang2/rule-error.gspec

@@ -0,0 +1,7 @@
+
+@rule %{item} is located at the %{side} side inside %{parentObject}
+    ${item}:
+        inside ${parentObject} 0px ${side}
+
+= Main =
+    |button is located at the left side inside main_container with 10px margin

galen-core/src/test/resources/speclang2/rule-priority.gspec

@@ -11,5 +11,15 @@
         width 300px
 
 
+@rule should be small
+    width 10px
+
+
+@rule should be small
+    width 30px
+
 = Main =
     |login_button should be big
+
+    cancel_button:
+        |should be small