You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@creadur.apache.org by "Claudenw (via GitHub)" <gi...@apache.org> on 2025/11/08 00:01:48 UTC
[PR] fix core tests [creadur-rat]
Claudenw opened a new pull request, #576:
URL: https://github.com/apache/creadur-rat/pull/576
Fixes for core tests.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-465: fix core tests after removal of deprecated objects [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger commented on code in PR #576:
URL: https://github.com/apache/creadur-rat/pull/576#discussion_r2513937106
##########
apache-rat-core/src/main/java/org/apache/rat/Report.java:
##########
@@ -48,19 +49,23 @@ public static void main(final String[] args) throws Exception {
System.exit(0);
}
- ReportConfiguration configuration = OptionCollection.parseCommands(new File("."), args, Report::printUsage);
- if (configuration != null) {
- configuration.validate(DefaultLog.getInstance()::error);
- Reporter reporter = new Reporter(configuration);
- reporter.output();
- reporter.writeSummary(DefaultLog.getInstance().asWriter());
+ try {
+ ReportConfiguration configuration = OptionCollection.parseCommands(new File("."), args, Report::printUsage);
+ if (configuration != null) {
+ configuration.validate(DefaultLog.getInstance()::error);
+ Reporter reporter = new Reporter(configuration);
+ reporter.output();
+ reporter.writeSummary(DefaultLog.getInstance().asWriter());
- if (configuration.getClaimValidator().hasErrors()) {
- configuration.getClaimValidator().logIssues(reporter.getClaimsStatistic());
- throw new RatDocumentAnalysisException(format("Issues with %s",
- String.join(", ",
- configuration.getClaimValidator().listIssues(reporter.getClaimsStatistic()))));
+ if (configuration.getClaimValidator().hasErrors()) {
+ configuration.getClaimValidator().logIssues(reporter.getClaimsStatistic());
+ throw new RatDocumentAnalysisException(format("Issues with %s",
+ String.join(", ",
+ configuration.getClaimValidator().listIssues(reporter.getClaimsStatistic()))));
+ }
}
+ } catch (ParseException e) {
Review Comment:
Shouldn't we log something before exiting with status 1?
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-465: fix core tests after removal of deprecated objects [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger commented on PR #576:
URL: https://github.com/apache/creadur-rat/pull/576#issuecomment-3516513481
@Claudenw thanks for all your work - littel review comments attached. Let's see how things evolve while fixing the other modules! At least it simplifies many things and makes RAT cleaner. Thx
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-465: fix core tests after removal of deprecated objects [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger commented on code in PR #576:
URL: https://github.com/apache/creadur-rat/pull/576#discussion_r2513943094
##########
apache-rat-core/src/test/java/org/apache/rat/test/AbstractConfigurationOptionsProvider.java:
##########
@@ -665,7 +614,7 @@ protected void logLevelTest() {
args[0] = level.name();
generateConfig(ImmutablePair.of(option, args));
assertThat(DefaultLog.getInstance().getLevel()).isEqualTo(level);
- } catch (IOException e) {
+ } catch (IOException | ParseException e) {
Review Comment:
Couldn't we let ParseException extend IOException in order to simplify the code here?
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-465: fix core tests after removal of deprecated objects [creadur-rat]
Posted by "Claudenw (via GitHub)" <gi...@apache.org>.
Claudenw commented on code in PR #576:
URL: https://github.com/apache/creadur-rat/pull/576#discussion_r2516029070
##########
apache-rat-core/src/test/java/org/apache/rat/test/AbstractConfigurationOptionsProvider.java:
##########
@@ -665,7 +614,7 @@ protected void logLevelTest() {
args[0] = level.name();
generateConfig(ImmutablePair.of(option, args));
assertThat(DefaultLog.getInstance().getLevel()).isEqualTo(level);
- } catch (IOException e) {
+ } catch (IOException | ParseException e) {
Review Comment:
We don't own ParseExcepton. It is from commons cli.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-465: fix core tests after removal of deprecated objects [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger commented on code in PR #576:
URL: https://github.com/apache/creadur-rat/pull/576#discussion_r2513949989
##########
apache-rat-core/src/test/java/org/apache/rat/ReporterOptionsProvider.java:
##########
@@ -920,58 +877,53 @@ private void styleSheetTest(final Option option) {
String actualText = baos.toString(StandardCharsets.UTF_8.name());
TextUtils.assertContainsExactly(1, "Hello world", actualText);
- } catch (IOException | RatException e) {
+ } catch (IOException | ParseException | RatException e) {
fail(e.getMessage(), e);
} finally {
System.setOut(origin);
}
}
- @OptionCollectionTest.TestFunction
- protected void stylesheetTest() {
- styleSheetTest(Arg.OUTPUT_STYLE.find("stylesheet"));
- }
-
@OptionCollectionTest.TestFunction
protected void outputStyleTest() {
styleSheetTest(Arg.OUTPUT_STYLE.find("output-style"));
}
- @OptionCollectionTest.TestFunction
- protected void xmlTest() {
- PrintStream origin = System.out;
- Option option = Arg.OUTPUT_STYLE.find("xml");
- ByteArrayOutputStream baos = new ByteArrayOutputStream();
- try (PrintStream out = new PrintStream(baos)) {
- System.setOut(out);
- configureSourceDir(option);
- // create a dummy stylesheet so that we match the stylesheet tests.
- File file = writeFile("stylesheet", "<xsl:stylesheet version=\"1.0\" xmlns:xsl=\"http://www.w3.org/1999/XSL/Transform\">\n" +
- " <xsl:template match=\"@*|node()\">\n" +
- " Hello world\n" +
- " </xsl:template>\n" +
- "</xsl:stylesheet>");
-
- ReportConfiguration config = generateConfig(ImmutablePair.of(option, null));
- Reporter reporter = new Reporter(config);
- ClaimStatistic claimStatistic = reporter.output();
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.STANDARDS)).isEqualTo(1);
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.APPROVED)).isEqualTo(0);
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.UNAPPROVED)).isEqualTo(1);
-
- String actualText = baos.toString(StandardCharsets.UTF_8.name());
- TextUtils.assertContainsExactly(1, "<resource encoding=\"ISO-8859-1\" mediaType=\"text/plain\" name=\"/stylesheet\" type=\"STANDARD\">", actualText);
-
- try (InputStream expected = StyleSheets.getStyleSheet("xml").get();
- InputStream actual = config.getStyleSheet().get()) {
- assertThat(IOUtils.contentEquals(expected, actual)).as("'xml' does not match").isTrue();
- }
- } catch (IOException | RatException e) {
- fail(e.getMessage(), e);
- } finally {
- System.setOut(origin);
- }
- }
+// @OptionCollectionTest.TestFunction
Review Comment:
Could we add a TODO here in order to make it easier to find the spot that needs more work ;) Thx
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-465: fix core tests after removal of deprecated objects [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger commented on code in PR #576:
URL: https://github.com/apache/creadur-rat/pull/576#discussion_r2513950931
##########
apache-rat-core/src/test/java/org/apache/rat/ReporterOptionsProvider.java:
##########
@@ -798,37 +760,37 @@ protected void licensesApprovedTest() {
new String[]{"GPL1"});
}
- @OptionCollectionTest.TestFunction
- protected void scanHiddenDirectoriesTest() {
- Option option = Arg.INCLUDE_STD.find("scan-hidden-directories");
- Pair<Option, String[]> arg1 = ImmutablePair.of(option, null);
-
- try {
- configureSourceDir(option);
-
- writeFile("apl.txt", "SPDX-License-Identifier: Apache-2.0");
-
- File hiddenDir = new File(sourceDir, ".hiddendir");
- FileUtils.mkDir(hiddenDir);
- FileUtils.writeFile(hiddenDir, "gpl.txt", Collections.singletonList("SPDX-License-Identifier: GPL-1.0-only"));
-
- ReportConfiguration config = generateConfig();
- Reporter reporter = new Reporter(config);
- ClaimStatistic claimStatistic = reporter.execute();
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.STANDARDS)).isEqualTo(1);
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.APPROVED)).isEqualTo(1);
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.UNAPPROVED)).isEqualTo(0);
-
- config = generateConfig(arg1);
- reporter = new Reporter(config);
- claimStatistic = reporter.execute();
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.STANDARDS)).isEqualTo(2);
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.APPROVED)).isEqualTo(1);
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.UNAPPROVED)).isEqualTo(1);
- } catch (IOException | RatException e) {
- fail(e.getMessage(), e);
- }
- }
+// @OptionCollectionTest.TestFunction
Review Comment:
Could we add a TODO here in order to make it easier to find the spot that needs more work ;) Thx
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-465: fix core tests after removal of deprecated objects [creadur-rat]
Posted by "Claudenw (via GitHub)" <gi...@apache.org>.
Claudenw commented on code in PR #576:
URL: https://github.com/apache/creadur-rat/pull/576#discussion_r2516059917
##########
apache-rat-core/src/test/java/org/apache/rat/ReporterOptionsProvider.java:
##########
@@ -920,58 +877,53 @@ private void styleSheetTest(final Option option) {
String actualText = baos.toString(StandardCharsets.UTF_8.name());
TextUtils.assertContainsExactly(1, "Hello world", actualText);
- } catch (IOException | RatException e) {
+ } catch (IOException | ParseException | RatException e) {
fail(e.getMessage(), e);
} finally {
System.setOut(origin);
}
}
- @OptionCollectionTest.TestFunction
- protected void stylesheetTest() {
- styleSheetTest(Arg.OUTPUT_STYLE.find("stylesheet"));
- }
-
@OptionCollectionTest.TestFunction
protected void outputStyleTest() {
styleSheetTest(Arg.OUTPUT_STYLE.find("output-style"));
}
- @OptionCollectionTest.TestFunction
- protected void xmlTest() {
- PrintStream origin = System.out;
- Option option = Arg.OUTPUT_STYLE.find("xml");
- ByteArrayOutputStream baos = new ByteArrayOutputStream();
- try (PrintStream out = new PrintStream(baos)) {
- System.setOut(out);
- configureSourceDir(option);
- // create a dummy stylesheet so that we match the stylesheet tests.
- File file = writeFile("stylesheet", "<xsl:stylesheet version=\"1.0\" xmlns:xsl=\"http://www.w3.org/1999/XSL/Transform\">\n" +
- " <xsl:template match=\"@*|node()\">\n" +
- " Hello world\n" +
- " </xsl:template>\n" +
- "</xsl:stylesheet>");
-
- ReportConfiguration config = generateConfig(ImmutablePair.of(option, null));
- Reporter reporter = new Reporter(config);
- ClaimStatistic claimStatistic = reporter.output();
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.STANDARDS)).isEqualTo(1);
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.APPROVED)).isEqualTo(0);
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.UNAPPROVED)).isEqualTo(1);
-
- String actualText = baos.toString(StandardCharsets.UTF_8.name());
- TextUtils.assertContainsExactly(1, "<resource encoding=\"ISO-8859-1\" mediaType=\"text/plain\" name=\"/stylesheet\" type=\"STANDARD\">", actualText);
-
- try (InputStream expected = StyleSheets.getStyleSheet("xml").get();
- InputStream actual = config.getStyleSheet().get()) {
- assertThat(IOUtils.contentEquals(expected, actual)).as("'xml' does not match").isTrue();
- }
- } catch (IOException | RatException e) {
- fail(e.getMessage(), e);
- } finally {
- System.setOut(origin);
- }
- }
+// @OptionCollectionTest.TestFunction
Review Comment:
Code is removed as it was a duplicate.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-465: fix core tests after removal of deprecated objects [creadur-rat]
Posted by "Claudenw (via GitHub)" <gi...@apache.org>.
Claudenw commented on PR #576:
URL: https://github.com/apache/creadur-rat/pull/576#issuecomment-3519035095
If you are happy with the changes please merge this and I will get the maven one queued up for review.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-465: fix core tests after removal of deprecated objects [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger commented on code in PR #576:
URL: https://github.com/apache/creadur-rat/pull/576#discussion_r2513863544
##########
apache-rat-core/src/main/java/org/apache/rat/OptionCollection.java:
##########
@@ -104,20 +122,11 @@ public static ReportConfiguration parseCommands(final File workingDirectory, fin
* @throws IOException on error.
*/
public static ReportConfiguration parseCommands(final File workingDirectory, final String[] args,
- final Consumer<Options> helpCmd, final boolean noArgs) throws IOException {
+ final Consumer<Options> helpCmd, final boolean noArgs) throws IOException, ParseException {
Review Comment:
ParseException seems to be missing in Javadoc.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-465: fix core tests after removal of deprecated objects [creadur-rat]
Posted by "Claudenw (via GitHub)" <gi...@apache.org>.
Claudenw commented on code in PR #576:
URL: https://github.com/apache/creadur-rat/pull/576#discussion_r2516068420
##########
apache-rat-core/src/test/java/org/apache/rat/ReporterOptionsProvider.java:
##########
@@ -798,37 +760,37 @@ protected void licensesApprovedTest() {
new String[]{"GPL1"});
}
- @OptionCollectionTest.TestFunction
- protected void scanHiddenDirectoriesTest() {
- Option option = Arg.INCLUDE_STD.find("scan-hidden-directories");
- Pair<Option, String[]> arg1 = ImmutablePair.of(option, null);
-
- try {
- configureSourceDir(option);
-
- writeFile("apl.txt", "SPDX-License-Identifier: Apache-2.0");
-
- File hiddenDir = new File(sourceDir, ".hiddendir");
- FileUtils.mkDir(hiddenDir);
- FileUtils.writeFile(hiddenDir, "gpl.txt", Collections.singletonList("SPDX-License-Identifier: GPL-1.0-only"));
-
- ReportConfiguration config = generateConfig();
- Reporter reporter = new Reporter(config);
- ClaimStatistic claimStatistic = reporter.execute();
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.STANDARDS)).isEqualTo(1);
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.APPROVED)).isEqualTo(1);
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.UNAPPROVED)).isEqualTo(0);
-
- config = generateConfig(arg1);
- reporter = new Reporter(config);
- claimStatistic = reporter.execute();
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.STANDARDS)).isEqualTo(2);
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.APPROVED)).isEqualTo(1);
- assertThat(claimStatistic.getCounter(ClaimStatistic.Counter.UNAPPROVED)).isEqualTo(1);
- } catch (IOException | RatException e) {
- fail(e.getMessage(), e);
- }
- }
+// @OptionCollectionTest.TestFunction
Review Comment:
removed as it was duplicated code.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-465: fix core tests after removal of deprecated objects [creadur-rat]
Posted by "Claudenw (via GitHub)" <gi...@apache.org>.
Claudenw commented on PR #576:
URL: https://github.com/apache/creadur-rat/pull/576#issuecomment-3519038069
@ottlinger If you are happy with the changes please merge and I will get the maven one queued up for review.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org