Let consistency checker yield a return code (#13329)

* Let consistency checker yield a return code

See #13328

* Apply suggestions from code review

As suggested by @koppor it makes totally sense to always exit with a
non-zero exit code in case of issues.  Exit code 0 means no issues in
linting, exit code 1 means linting issues, exit code 2 means any other
issues.

Also remove the CLI flag to enable/disable the feature.

* Reflect the changes in CHANGELOG.md

Author: stephanlukasczyk

CHANGELOG.md

@@ -46,6 +46,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 - We improved JabRef's internal document viewer. It now allows text section, searching and highlighting of search terms and page rotation [#13193](https://github.com/JabRef/jabref/pull/13193).
 - When importing a PDF, there is no empty entry column shown in the multi merge dialog. [#13132](https://github.com/JabRef/jabref/issues/13132)
 - We added a progress dialog to the "Check consistency" action and progress output to the corresponding cli command. [#12487](https://github.com/JabRef/jabref/issues/12487)
+- We made the `check-consistency` command of the toolkit always return an exit code; 0 means no issues found, a non-zero exit code reflects any issues, which allows CI to fail in these cases [#13328](https://github.com/JabRef/jabref/issues/13328).
 
 ### Fixed
 

jabkit/src/main/java/org/jabref/JabKit.java

@@ -85,7 +85,8 @@ public static void main(String[] args) {
                     ArgumentProcessor.getAvailableImportFormats(preferences),
                     ArgumentProcessor.getAvailableExportFormats(preferences),
                     WebFetchers.getSearchBasedFetchers(preferences.getImportFormatPreferences(), preferences.getImporterPreferences()));
-            commandLine.execute(args);
+            int result = commandLine.execute(args);
+            System.exit(result);
         } catch (Exception ex) {
             LOGGER.error("Unexpected exception", ex);
         }

jabkit/src/main/java/org/jabref/cli/CheckConsistency.java

@@ -5,6 +5,7 @@
 import java.io.Writer;
 import java.util.List;
 import java.util.Optional;
+import java.util.concurrent.Callable;
 
 import org.jabref.logic.importer.ParserResult;
 import org.jabref.logic.l10n.Localization;
@@ -24,7 +25,7 @@
 import static picocli.CommandLine.ParentCommand;
 
 @Command(name = "check-consistency", description = "Check consistency of the library.")
-class CheckConsistency implements Runnable {
+class CheckConsistency implements Callable<Integer> {
     private static final Logger LOGGER = LoggerFactory.getLogger(CheckConsistency.class);
 
     @ParentCommand
@@ -40,20 +41,20 @@ class CheckConsistency implements Runnable {
     private String outputFormat;
 
     @Override
-    public void run() {
+    public Integer call() {
         Optional<ParserResult> parserResult = ArgumentProcessor.importFile(
                 inputFile,
                 "bibtex",
                 argumentProcessor.cliPreferences,
                 sharedOptions.porcelain);
         if (parserResult.isEmpty()) {
             System.out.println(Localization.lang("Unable to open file '%0'.", inputFile));
-            return;
+            return 2;
         }
 
         if (parserResult.get().isInvalid()) {
             System.out.println(Localization.lang("Input file '%0' is invalid and could not be parsed.", inputFile));
-            return;
+            return 2;
         }
 
         if (!sharedOptions.porcelain) {
@@ -95,11 +96,16 @@ public void run() {
             writer.flush();
         } catch (IOException e) {
             LOGGER.error("Error writing results", e);
-            return;
+            return 2;
+        }
+
+        if (!result.entryTypeToResultMap().isEmpty()) {
+            return 1;
         }
 
         if (!sharedOptions.porcelain) {
             System.out.println(Localization.lang("Consistency check completed"));
         }
+        return 0;
     }
 }

jabkit/src/test/java/org/jabref/cli/ArgumentProcessorTest.java

@@ -143,11 +143,12 @@ void checkConsistency() throws URISyntaxException {
         ByteArrayOutputStream outContent = new ByteArrayOutputStream();
         System.setOut(new PrintStream(outContent, true));
 
-        commandLine.execute(args.toArray(String[]::new));
+        int executionResult = commandLine.execute(args.toArray(String[]::new));
 
         String output = outContent.toString();
         assertTrue(output.contains("Checking consistency for entry type 1 of 1\n"));
         assertTrue(output.contains("Consistency check completed"));
+        assertEquals(0, executionResult);
 
         System.setOut(System.out);
     }
@@ -163,10 +164,11 @@ void checkConsistencyPorcelain() throws URISyntaxException {
         ByteArrayOutputStream outContent = new ByteArrayOutputStream();
         System.setOut(new PrintStream(outContent));
 
-        commandLine.execute(args.toArray(String[]::new));
+        int executionResult = commandLine.execute(args.toArray(String[]::new));
 
         String output = outContent.toString();
         assertEquals("", output);
+        assertEquals(0, executionResult);
 
         System.setOut(System.out);
     }