[Watcher] Improved error messages for CronEvalTool

CronEvalTool prints an error only for cron expressions that result in
no coming time events.

If a cron expression results in less than the specified `-count`
(default 10) time events, the CronEvalTool prints all the coming
times and displays no error message.

Fixes elastic#32735

Author: lipsill

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/tool/CronEvalTool.java

@@ -5,6 +5,7 @@
  */
 package org.elasticsearch.xpack.watcher.trigger.schedule.tool;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
@@ -57,18 +58,25 @@ void execute(Terminal terminal, String expression, int count) throws Exception {
 
         DateTime date = DateTime.now(DateTimeZone.UTC);
         terminal.println("Now is [" + formatter.print(date) + "]");
-        terminal.println("Here are the next " + count + " times this cron expression will trigger:");
 
         Cron cron = new Cron(expression);
         long time = date.getMillis();
-        for (int i = 0; i < count; i++) {
+        int i = 0;
+        List<String> times = new ArrayList<>();
+
+        for (; i < count; i++) {
             long prevTime = time;
             time = cron.getNextValidTimeAfter(time);
             if (time < 0) {
-                throw new UserException(ExitCodes.OK, (i + 1) + ".\t Could not compute future times since ["
-                    + formatter.print(prevTime) + "] " + "(perhaps the cron expression only points to times in the past?)");
+                if (i == 0) {
+                    throw new UserException(ExitCodes.OK, "Could not compute future times since ["
+                            + formatter.print(prevTime) + "] " + "(perhaps the cron expression only points to times in the past?)");
+                }
+                break;
             }
-            terminal.println((i+1) + ".\t" + formatter.print(time));
+            times.add((i + 1) + ".\t" + formatter.print(time));
         }
+        terminal.println((i == count ? "Here are the next " : "There are ") + i + " times this cron expression will trigger:");
+        times.forEach(terminal::println);
     }
 }

x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/tool/CronEvalToolTests.java

@@ -8,6 +8,10 @@
 import org.elasticsearch.cli.Command;
 import org.elasticsearch.cli.CommandTestCase;
 
+import java.util.Calendar;
+import java.util.Locale;
+import java.util.TimeZone;
+
 public class CronEvalToolTests extends CommandTestCase {
     @Override
     protected Command newCommand() {
@@ -20,4 +24,28 @@ public void testParse() throws Exception {
         String output = execute(countOption, Integer.toString(count), "0 0 0 1-6 * ?");
         assertTrue(output, output.contains("Here are the next " + count + " times this cron expression will trigger"));
     }
+
+    public void testGetNextValidTimes() throws Exception {
+        {
+            String output = execute(
+                    "0 3 23 8 9 ? " + (Calendar.getInstance(TimeZone.getTimeZone("UTC"), Locale.ROOT).get(Calendar.YEAR) + 1));
+            assertTrue(output, output.contains("There are 1 times this cron expression will trigger:"));
+            assertFalse(output.contains("ERROR"));
+            assertFalse(output.contains("2.\t"));
+        }
+        {
+            String output = execute(
+                    "0 3 23 */4 9 ? " + (Calendar.getInstance(TimeZone.getTimeZone("UTC"), Locale.ROOT).get(Calendar.YEAR) + 1));
+            assertTrue(output, output.contains("There are 8 times this cron expression will trigger:"));
+            assertFalse(output, output.contains("Here are the next 10 times this cron expression will trigger:"));
+            assertFalse(output, output.contains("ERROR"));
+        }
+        {
+            Exception expectThrows = expectThrows(Exception.class, () -> execute(
+                    "0 3 23 */4 9 ? " + (Calendar.getInstance(TimeZone.getTimeZone("UTC"), Locale.ROOT).get(Calendar.YEAR) - 1)));
+            String message = expectThrows.getMessage();
+            assertTrue(message, message.contains("Could not compute future times since"));
+            assertTrue(message, message.contains("(perhaps the cron expression only points to times in the past?)"));
+        }
+    }
 }