#419 Fix inconsistencies in documentation and error messages (#422)

* #419 Fix inconsistencies in documentation and error messages * #419: Add tests for inconsistent tag parsing, see #423 --------- Co-authored-by: kaklakariada <[email protected]>
itsallcode · Jul 6, 2024 · 7b26b0e · 7b26b0e
1 parent 0e4a88d
commit 7b26b0e
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 30 deletions.
diff --git a/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CommandLineInterpreter.java b/core/src/main/java/org/itsallcode/openfasttrace/core/cli/CommandLineInterpreter.java
@@ -101,7 +101,7 @@ else if (argument.startsWith(SINGLE_CHAR_ARG_PREFIX))
     private void handleChainedSingleCharacterArguments(final ListIterator<String> iterator,
             final String argument) throws CliException
     {
-        final String characters = argument.replaceFirst(SINGLE_CHAR_ARG_PREFIX, "").toLowerCase();
+        final String characters = argument.replaceFirst(SINGLE_CHAR_ARG_PREFIX, "").toLowerCase(Locale.ENGLISH);
         final int lastPosition = characters.length() - 1;
 
         for (int position = 0; position <= lastPosition; ++position)
@@ -120,26 +120,26 @@ private void handleChainedSingleCharacterArguments(final ListIterator<String> it
             }
             else
             {
-                reportUnexpectedNamedArgument(character);
+                reportUnexpectedNamedArgument(argument);
             }
         }
     }
 
     private void handleNamedArgument(final ListIterator<String> iterator, final String argument)
             throws CliException
     {
-        final String argumentName = argument.replace("-", "").toLowerCase();
+        final String argumentName = argument.replace("-", "").toLowerCase(Locale.ENGLISH);
         if (this.setters.containsKey(argumentName))
         {
             handleExpectedNamedArgument(iterator, argumentName);
         }
         else
         {
-            reportUnexpectedNamedArgument(argumentName);
+            reportUnexpectedNamedArgument(argument);
         }
     }
 
-    private void handleUnnamedArgument(final List<String> unnamedArguments, final String argument)
+    private static void handleUnnamedArgument(final List<String> unnamedArguments, final String argument)
     {
         unnamedArguments.add(argument);
     }
@@ -167,10 +167,10 @@ private void handleExpectedNamedArgument(final ListIterator<String> iterator,
             if ((iterator != null) && iterator.hasNext())
             {
                 final String successor = iterator.next();
-                if (isParamterName(successor))
+                if (isParameterName(successor))
                 {
                     iterator.previous();
-                    reportMissingParamterValue(argumentName);
+                    reportMissingParameterValue(argumentName);
                 }
                 else
                 {
@@ -180,7 +180,7 @@ private void handleExpectedNamedArgument(final ListIterator<String> iterator,
             }
             else
             {
-                reportMissingParamterValue(argumentName);
+                reportMissingParameterValue(argumentName);
             }
         }
     }
@@ -207,7 +207,7 @@ private <T> T convertEnum(final String stringValue, final Class<T> type) throws
         try
         {
             @SuppressWarnings("rawtypes")
-            final Enum enumValue = Enum.valueOf(enumType, stringValue.toUpperCase());
+            final Enum enumValue = Enum.valueOf(enumType, stringValue.toUpperCase(Locale.ENGLISH));
             return type.cast(enumValue);
         }
         catch (final IllegalArgumentException e)
@@ -224,12 +224,12 @@ private void reportUnsupportedSetterArgumentCount(final Method setter) throws Cl
                 + "'. Only one argument is allowed.");
     }
 
-    private void reportMissingParamterValue(final String argumentName) throws CliException
+    private void reportMissingParameterValue(final String argumentName) throws CliException
     {
         throw new CliException("No value for argument '" + argumentName + "'");
     }
 
-    private boolean isParamterName(final String text)
+    private static boolean isParameterName(final String text)
     {
         return text.startsWith(SINGLE_CHAR_ARG_PREFIX);
     }

diff --git a/core/src/test/java/org/itsallcode/openfasttrace/core/cli/TestCommandLineInterpreter.java b/core/src/test/java/org/itsallcode/openfasttrace/core/cli/TestCommandLineInterpreter.java
@@ -10,6 +10,8 @@
 
 import org.itsallcode.openfasttrace.core.cli.CommandLineArgumentsStub.StubEnum;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 /**
  * Tests for {@link CommandLineInterpreter}
@@ -44,18 +46,14 @@ void testMissingValueForStringParameter()
                 "No value for argument 'a'");
     }
 
-    @Test
-    void testUnexpectedArgumentName()
-    {
-        expectParseException(new CommandLineArgumentsStub(), asList("--unexpected"),
-                "Unexpected parameter 'unexpected' is not allowed");
-    }
-
-    @Test
-    void testUnexpectedSingleCharacterArgumentName()
+    @ParameterizedTest
+    @ValueSource(strings =
+    { "--unexpected", "--Unexpected", "-u", "--u", "--long-unexpected-parameter", "-unexpectedParameter",
+            "--unexpectedParameter" })
+    void testUnexpectedArgumentName(final String parameter)
     {
-        expectParseException(new CommandLineArgumentsStub(), asList("-u"),
-                "Unexpected parameter 'u' is not allowed");
+        expectParseException(new CommandLineArgumentsStub(), asList(parameter),
+                "Unexpected parameter '" + parameter + "' is not allowed");
     }
 
     @Test

diff --git a/doc/changes/changes_4.0.2.md b/doc/changes/changes_4.0.2.md
@@ -6,6 +6,13 @@ Code name: Fix usability of HTML report
 
 This release updates the HTML report so that jumping to anchored tags does no longer hide the target section under the navbar. Thanks to [@RobertZickler](https://github.com/RobertZickler) for fixing this!
 
+The release also fixes some inconsistencies in the documentation and in error messages reported by [@RobertZickler](https://github.com/RobertZickler):
+* OFT now reports the exact name for unexpected parameters instead of removing dashes and converting it to lower case.
+* The user guide now also lists `aspec` as possible option for `--output-format`
+* The user guide now shows the correct option `--report-verbosity` instead of `--verbosity-level`
+* The user guide example command line for generating an Aspec tracing report fixes a typo in the reporter name `aspac` instead of `aspec`
+
 ## Bugfixes
 
 * #420: Fix jumping in HTML reports ([@RobertZickler](https://github.com/RobertZickler))
+* #419: Fixed inconsistencies in documentation and error messages (reported by [@RobertZickler](https://github.com/RobertZickler))
diff --git a/doc/user_guide.md b/doc/user_guide.md
@@ -330,7 +330,7 @@ is functionally equivalent to
 
 ##### `Tags`
 
-Tags are described in detail later in this document.
+Tags are described in detail later in this document, see section [Distributing the Detailing Work](#distributing-the-detailing-work).
 
 ### Delegating Requirement Coverage
 
@@ -354,7 +354,7 @@ This notation can appear after:
 * "Covers" section
 * "Tags" section
 
-If it appears in a mutli-line text section of a requirement (description, comment or rationale) the forward is ignored.
+If it appears in a multi-line text section of a requirement (description, comment or rationale) the forward is ignored.
 
 Note that a forward terminates the previous specification item, so the following notation does not work:
 
@@ -495,11 +495,12 @@ The format of the report.
 
 One of:
 * `plain`
-* `html` 
+* `html`
+* `aspec`
 
 Defaults to `plain`.
 
-    --v, --verbosity-level <level>
+    --v, --report-verbosity <level>
 
 The verbosity of the tracing report.
 
@@ -770,7 +771,7 @@ The XML exporter is called `aspec` reporter. `aspec` in this case means augmente
 can be generated  by calling OpenFastTrace in the following way:
 
 ```bash
-java -jar openfasttrace.jar trace -o aspac -f requirements.xml requirements
+java -jar openfasttrace.jar trace -o aspec -f requirements.xml requirements
 ```
 
 OpenFastTrace needs to be executed  with the command `trace` to activate the reporter. The `aspec` report is selected 

diff --git a/...wn/src/test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownImporter.java b/...wn/src/test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownImporter.java
@@ -88,7 +88,8 @@ void testIdentifyTags(final String text)
 
     @ParameterizedTest
     @CsvSource(
-    { "Tags:", "#Needs: abc", "' Needs: abc'", "Needs: änderung" })
+    { "Tags:", "#Needs: abc", "' Needs: abc'", "Needs: änderung", "Tags: -leadingDash", "Tags: trailingDash-",
+            "Tags: tag with spaces", "Tags: Täg" })
     void testIdentifyNonTags(final String text)
     {
         MarkdownAsserts.assertMismatch(MdPattern.TAGS_INT, text);

diff --git a/.../test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownMarkupImporter.java b/.../test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownMarkupImporter.java
@@ -12,7 +12,7 @@
 
 class TestMarkdownMarkupImporter extends AbstractLightWeightMarkupImporterTest
 {
-    private final static ImporterFactory importerFactory = new MarkdownImporterFactory();
+    private static final ImporterFactory importerFactory = new MarkdownImporterFactory();
 
     TestMarkdownMarkupImporter()
     {

diff --git a/...nfasttrace/testutil/importer/lightweightmarkup/AbstractLightWeightMarkupImporterTest.java b/...nfasttrace/testutil/importer/lightweightmarkup/AbstractLightWeightMarkupImporterTest.java
@@ -163,6 +163,7 @@ static Stream<Arguments> tags()
         return Stream.of(
                 Arguments.of("Tags: req , dsn ", List.of("req", "dsn")),
                 Arguments.of("Tags: req ", List.of("req")),
+                Arguments.of("Tags: req_1 ", List.of("req_1")),
                 Arguments.of("Tags: req,dsn ", List.of("req", "dsn")),
                 Arguments.of("Tags: req ,dsn", List.of("req", "dsn")),
                 Arguments.of("Tags: req,dsn", List.of("req", "dsn")),
@@ -171,7 +172,17 @@ static Stream<Arguments> tags()
                 Arguments.of("Tags:\n* req\n* dsn", List.of("req", "dsn")),
                 Arguments.of("Tags:\n * req\n * dsn\n", List.of("req", "dsn")),
                 Arguments.of("Tags:\n* req \n\t* dsn ", List.of("req", "dsn")),
-                Arguments.of("Tags:\n* req\n* dsn", List.of("req", "dsn")));
+                Arguments.of("Tags:\n* req\n* dsn", List.of("req", "dsn")),
+                Arguments.of("Tags:\n* req_1\n* dsn_2", List.of("req_1", "dsn_2")),
+
+                // Inconsistent behavior, see
+                // https://github.com/itsallcode/openfasttrace/issues/423
+                Arguments.of("Tags: _ ", List.of("_")),
+                Arguments.of("Tags: täg1, taeg2", List.of()),
+                Arguments.of("Tags: _tag1, taeg2", List.of("_tag1", "taeg2")),
+                Arguments.of("Tags: -dash, tag", List.of()),
+                Arguments.of("Tags:\n* tag1\n* täg2", List.of("tag1", "täg2")),
+                Arguments.of("Tags:\n* -tag1\n* täg2", List.of("-tag1", "täg2")));
     }
 
     @ParameterizedTest