ottlinger commented on code in PR #268: URL: https://github.com/apache/creadur-rat/pull/268#discussion_r1667733329
########## apache-rat-core/src/test/java/org/apache/rat/OptionCollectionTest.java: ########## @@ -118,27 +121,44 @@ public void testDeprecatedUseLogged() throws IOException { TestingLog log = new TestingLog(); try { DefaultLog.setInstance(log); - String[] args = {longOpt(OptionCollection.DIR), "foo", "-a"}; - ReportConfiguration config = OptionCollection.parseCommands(args, (o) -> { - }, true); - + String[] args = {longOpt(OptionCollection.DIR), "target", "-a"}; + ReportConfiguration config = OptionCollection.parseCommands(args, o -> fail("Help printed"), true); } finally { DefaultLog.setInstance(null); } log.assertContains("WARN: Option [-d, --dir] used. Deprecated for removal since 0.17: Use '--'"); + log.assertNotContains("WARN: Option [-d, --dir] used. Deprecated for removal since 0.17: Use '--'", 1); log.assertContains("WARN: Option [-a] used. Deprecated for removal since 0.17: Use '-A' or '--addLicense'"); + log.assertNotContains("WARN: Option [-a] used. Deprecated for removal since 0.17: Use '-A' or '--addLicense'", 1); + } + + @Test + public void testDirOptionCapturesDirectoryToScan() throws IOException { + TestingLog log = new TestingLog(); + ReportConfiguration config = null; + try { + DefaultLog.setInstance(log); + String[] args = {longOpt(OptionCollection.DIR), "foo"}; + config = OptionCollection.parseCommands(args, (o) -> { + }, true); + } finally { + DefaultLog.setInstance(null); + } + assertThat(config).isNotNull(); + log.assertContains("WARN: Option [-d, --dir] used. Deprecated for removal since 0.17: Use '--'"); + log.assertNotContains("WARN: Option [-d, --dir] used. Deprecated for removal since 0.17: Use '--'", 1); Review Comment: Should we merge both assertions into one or only use exactly one? ########## apache-rat-core/src/test/java/org/apache/rat/OptionCollectionTest.java: ########## @@ -118,27 +121,44 @@ public void testDeprecatedUseLogged() throws IOException { TestingLog log = new TestingLog(); try { DefaultLog.setInstance(log); - String[] args = {longOpt(OptionCollection.DIR), "foo", "-a"}; - ReportConfiguration config = OptionCollection.parseCommands(args, (o) -> { - }, true); - + String[] args = {longOpt(OptionCollection.DIR), "target", "-a"}; + ReportConfiguration config = OptionCollection.parseCommands(args, o -> fail("Help printed"), true); } finally { DefaultLog.setInstance(null); } log.assertContains("WARN: Option [-d, --dir] used. Deprecated for removal since 0.17: Use '--'"); + log.assertNotContains("WARN: Option [-d, --dir] used. Deprecated for removal since 0.17: Use '--'", 1); log.assertContains("WARN: Option [-a] used. Deprecated for removal since 0.17: Use '-A' or '--addLicense'"); + log.assertNotContains("WARN: Option [-a] used. Deprecated for removal since 0.17: Use '-A' or '--addLicense'", 1); + } + + @Test + public void testDirOptionCapturesDirectoryToScan() throws IOException { + TestingLog log = new TestingLog(); + ReportConfiguration config = null; + try { + DefaultLog.setInstance(log); + String[] args = {longOpt(OptionCollection.DIR), "foo"}; + config = OptionCollection.parseCommands(args, (o) -> { + }, true); + } finally { + DefaultLog.setInstance(null); + } + assertThat(config).isNotNull(); + log.assertContains("WARN: Option [-d, --dir] used. Deprecated for removal since 0.17: Use '--'"); + log.assertNotContains("WARN: Option [-d, --dir] used. Deprecated for removal since 0.17: Use '--'", 1); Review Comment: Is the semantics of both lines supposed to be doesNotContainStringMoreThanExactlyOnce? Reading it one by one seems a bit confusing to me ..... sry -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org