Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tests for Windows EOL check #1236

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

k3p7i3
Copy link
Contributor

@k3p7i3 k3p7i3 commented Mar 17, 2024

Fixed tests for Windows EOL check.
Also, refactored CheckstyleValidatorTest a little bit to make it more flexible for tests with their own licences.

Closes: #1127

@k3p7i3
Copy link
Contributor Author

k3p7i3 commented Mar 19, 2024

@yegor256
This pull request doesn't pass checks because of the testWindowsEndsOfLineWithLinuxSources test.
The test is for the case when the license text is in crlf format, and the source code is in lf format, and the validation should fail.

When I run the test locally on Windows, it passes, but on Linux, it fails. In the screenshots, you can see that even the license text is parsed differently.
Screenshot_20240319_184406
photo_2024-03-19_19-21-01
In both cases, the license is created with eol="\r\n", and the source code is in lf format, but on Linux, HeaderCheck from checkstyle does not see the difference.
Unfortunately, I have not been able to find a solution to this problem. Perhaps you know how this can be resolved?

@yegor256
Copy link
Owner

@pnatashap maybe you know how to help?

@pnatashap
Copy link
Contributor

@k3p7i3 @yegor256 the reason is by default files after checkout will have os-dependent end-of-line symbol (but stored as linux styled in git). So the best variant is to change symbol in case of non-windows os (tests are run on ubuntu and mac) before run validation (so we will check all variants).

also there is an option to enable test only for windows (@EnabledOnOs(WINDOWS))

@k3p7i3
Copy link
Contributor Author

k3p7i3 commented Mar 20, 2024

@pnatashap Thanks for the advice! I tried to change the end-of-line symbol, but it didn't work out for some reason (at least in my local linux environment). So I used the second option.

@k3p7i3
Copy link
Contributor Author

k3p7i3 commented Mar 20, 2024

@yegor256 please take a look

final String message = "Line does not match expected header line of ' *'";
final String name = "HeaderCheck";
MatcherAssert.assertThat(
results,
Copy link
Contributor

Choose a reason for hiding this comment

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

please add reason for assertThat

MatcherAssert.assertThat(
results,
Matchers.contains(
new ViolationMatcher(message, file, "3", name)
Copy link
Contributor

Choose a reason for hiding this comment

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

A can not match test name and validation in it:
test is about windowEndsOfLine
validation is about HeaderCheck.

Matchers.contains(
new ViolationMatcher(
"Expected line ending for file is LF(\\n), but CRLF(\\r\\n) is detected",
file, "1", "NewlineAtEndOfFileCheck"
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks strange for me, that file has 10 lines of code and we are expecting NewlineAtEndOfFileCheck at line 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CheckstyleValidatorTest.java:446-448: This test and...
3 participants