Skip to content

RAT-559: fix DocumentName issues in archives/on Windows#674

Merged
Claudenw merged 13 commits into
masterfrom
fix-DocumentName-issues
Jun 25, 2026
Merged

RAT-559: fix DocumentName issues in archives/on Windows#674
Claudenw merged 13 commits into
masterfrom
fix-DocumentName-issues

Conversation

@Claudenw

@Claudenw Claudenw commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Fix for RAT-559

fixed DocumentName and DocumentName.Builder and added extensive testing to validate correctness.

@Claudenw Claudenw self-assigned this Jun 9, 2026
@Claudenw Claudenw added bug java Pull requests that update Java code labels Jun 9, 2026
@ottlinger ottlinger changed the title fix DocumentName issues RAT-559: fix DocumentName issues in archives/on Windows Jun 9, 2026
@Claudenw Claudenw marked this pull request as draft June 9, 2026 13:59
@ottlinger

ottlinger commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@Claudenw test failure seem to address the problem you tried to self:

Expecting value to be true but was false
Error:    ReporterTest.testXMLOutput:223 Could not find exactly one/rat-report/resource[@name='/.hiddenDirectory'] ==> expected: <1> but was: <0>
Error:    ArgTests.outputFileNameNoDirectoryTest:65 
expected: "D:\a\creadur-rat\creadur-rat\apache-rat-core\rat.txt"
 but was: "C:\a\creadur-rat\creadur-rat\apache-rat-core\rat.txt"
Error:    ArgTests.outputFileNameNoDirectoryTest:65 
expected: "D:\a\creadur-rat\creadur-rat\apache-rat-core\rat.txt"
 but was: "C:\a\creadur-rat\creadur-rat\apache-rat-core\rat.txt"
Error:    ArgTests.outputFileNameNoDirectoryTest:65 
expected: "D:\a\creadur-rat\creadur-rat\apache-rat-core\target\rat.test"
 but was: "C:\a\creadur-rat\creadur-rat\apache-rat-core\target\rat.test"
Error:    GitIgnoreBuilderTest.test_RAT_335:137 
expected: "matcherSet(or('included dir1/.gitignore', 'included .gitignore'), or('excluded dir1/.gitignore', **/.gitignore, 'excluded .gitignore'))"
 but was: "not(**/.gitignore)"
Error:    GitIgnoreBuilderTest.test_global_gitignore:175 
expected: "matcherSet(or('included dir1/.gitignore', 'included .gitignore', 'included global gitignore'), or('excluded dir1/.gitignore', **/.gitignore, 'excluded .gitignore', 'excluded global gitignore'))"
 but was: "not(**/.gitignore)"
Error:    XmlReportFactoryTest.standardReport:94 Standard files ==> expected: <8> but was: <9>
Error:  Errors: 
Error:    ReportTest.integrationTest(String, Document) � TemplateInvocationValidation Configuration error: You must configure at least one set of arguments for this @ParameterizedTest

Claudenw added 3 commits June 16, 2026 16:00
@Claudenw
checkpoint
9814d8f
@Claudenw
Merge branch 'master' into fix-DocumentName-issues
bac48e9 
# Conflicts:
#	apache-rat-core/src/main/java/org/apache/rat/configuration/XMLConfigurationReader.java
@Claudenw
Fixed several parsing errors in DocumentName.Builder
00f3079 
Add testing
Expanded testing to include multiple file system types
Fix recursion issue in DocumentName equals and compareTo
Additional migration to assertj
Changed File options to return DocumentName instances.
Cleaned up Sonar review issues
@Claudenw Claudenw force-pushed the fix-DocumentName-issues branch from 62d2521 to 00f3079 Compare June 22, 2026 07:13
@Claudenw Claudenw marked this pull request as ready for review June 22, 2026 07:14
@Claudenw Claudenw requested a review from ottlinger June 22, 2026 07:33
@Claudenw

Claudenw commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@ottlinger when you get a chance would you review this please.

ottlinger
ottlinger requested changes Jun 24, 2026
View reviewed changes

@ottlinger ottlinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to review my changes - the onliest thing to clarify would be if we need this assertDoNotThrow() at all. The rest is covered by tests and proven/secured against regressions.

Thanks for the PR!

Comment thread apache-rat-core/src/test/java/org/apache/rat/test/AbstractConfigurationOptionsProvider.java
String[] excluded = {"some.foo", "B.bar", "justbaz"};
try {

assertDoesNotThrow(() -> {

@ottlinger ottlinger Jun 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Claudenw I honestly did not really get why there were these try/catch before .... if the test fails an exception should be logged properly. Not sure if all this assertDoesNotThrow() makes sense at all, but it makes logging an exception more explicit. Any particular reason for these constructs?

@Claudenw Claudenw Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are used in the OptionTest implementations, so they have to either fail() or fail an assert.

Also, I think this was originally coded before assertDoesNotThrow was available.

@sonarqubecloud

sonarqubecloud Bot commented Jun 24, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
87.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@Claudenw Claudenw merged commit d8ba8f2 into master Jun 25, 2026
27 checks passed
@Claudenw Claudenw deleted the fix-DocumentName-issues branch June 25, 2026 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ottlinger ottlinger ottlinger requested changes

Assignees

@Claudenw Claudenw

Labels

bug java Pull requests that update Java code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Claudenw @ottlinger