Merge #986 'Use Integer.compare() in HookComparator'

Also Update History.md.

Author: brasmusson

History.md

@@ -1,5 +1,6 @@
 ## [1.2.5-SNAPSHOT](https://github.com/cucumber/cucumber-jvm/compare/v1.2.4...master) (In Git)
 
+* [Core] Use Integer.compare() in HookComparator in order to guard against possible underflow ([#986](https://github.com/cucumber/cucumber-jvm/pull/986), [#985](https://github.com/cucumber/cucumber-jvm/issues/985) Mikael Auno)
 * [Junit] Let JUnitReporter treat Pending results in hooks as failures in strict mode, and as ignored tests otherwise (Björn Rasmusson)
 * [Core] Mark scenario as skipped in JUnitFormatter if PendingException is thrown in a hook ([#964](https://github.com/cucumber/cucumber-jvm/pull/964), [#962](https://github.com/cucumber/cucumber-jvm/issues/962) Felix Martin Martin)
 * [Core] Support assume feature also with JUnit 4.12 ([#961](https://github.com/cucumber/cucumber-jvm/pull/961) Stefan Birkner)

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;
     }
 }

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());
     }