Use Integer.compare() in HookComparator in order to guard against pos…

…sible underflow
cucumber · Apr 6, 2016 · a696674 · a696674
1 parent 361c13c
commit a696674
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 4 deletions.
diff --git a/core/src/main/java/cucumber/runtime/HookComparator.java b/core/src/main/java/cucumber/runtime/HookComparator.java
@@ -11,7 +11,7 @@ public HookComparator(boolean ascending) {
 
     @Override
     public int compare(HookDefinition hook1, HookDefinition hook2) {
-        int comparison = hook1.getOrder() - hook2.getOrder();
+        int comparison = Integer.compare(hook1.getOrder(), hook2.getOrder());
         return ascending ? comparison : -comparison;
     }
 }
diff --git a/core/src/test/java/cucumber/runtime/HookOrderTest.java b/core/src/test/java/cucumber/runtime/HookOrderTest.java
@@ -37,31 +37,39 @@ public void buildMockWorld() {
 
     @Test
     public void before_hooks_execute_in_order() throws Throwable {
-        List<HookDefinition> hooks = mockHooks(3, Integer.MAX_VALUE, 1);
+        List<HookDefinition> hooks = mockHooks(3, Integer.MAX_VALUE, 1, -1, 0, 10000, Integer.MIN_VALUE);
         for (HookDefinition hook : hooks) {
             glue.addBeforeHook(hook);
         }
 
         runtime.runBeforeHooks(mock(Reporter.class), new HashSet<Tag>());
 
         InOrder inOrder = inOrder(hooks.toArray());
+        inOrder.verify(hooks.get(6)).execute(Matchers.<Scenario>any());
+        inOrder.verify(hooks.get(3)).execute(Matchers.<Scenario>any());
+        inOrder.verify(hooks.get(4)).execute(Matchers.<Scenario>any());
         inOrder.verify(hooks.get(2)).execute(Matchers.<Scenario>any());
         inOrder.verify(hooks.get(0)).execute(Matchers.<Scenario>any());
+        inOrder.verify(hooks.get(5)).execute(Matchers.<Scenario>any());
         inOrder.verify(hooks.get(1)).execute(Matchers.<Scenario>any());
     }
 
     @Test
     public void after_hooks_execute_in_reverse_order() throws Throwable {
-        List<HookDefinition> hooks = mockHooks(2, Integer.MAX_VALUE, 4);
+        List<HookDefinition> hooks = mockHooks(Integer.MIN_VALUE, 2, Integer.MAX_VALUE, 4, -1, 0, 10000);
         for (HookDefinition hook : hooks) {
             glue.addAfterHook(hook);
         }
 
         runtime.runAfterHooks(mock(Reporter.class), new HashSet<Tag>());
 
         InOrder inOrder = inOrder(hooks.toArray());
-        inOrder.verify(hooks.get(1)).execute(Matchers.<Scenario>any());
         inOrder.verify(hooks.get(2)).execute(Matchers.<Scenario>any());
+        inOrder.verify(hooks.get(6)).execute(Matchers.<Scenario>any());
+        inOrder.verify(hooks.get(3)).execute(Matchers.<Scenario>any());
+        inOrder.verify(hooks.get(1)).execute(Matchers.<Scenario>any());
+        inOrder.verify(hooks.get(5)).execute(Matchers.<Scenario>any());
+        inOrder.verify(hooks.get(4)).execute(Matchers.<Scenario>any());
         inOrder.verify(hooks.get(0)).execute(Matchers.<Scenario>any());
     }