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

Tests failing on Windows #3183

Open
Bananeweizen opened this issue Sep 15, 2023 · 3 comments · May be fixed by #3185
Open

Tests failing on Windows #3183

Bananeweizen opened this issue Sep 15, 2023 · 3 comments · May be fixed by #3185
Assignees
Labels
status: pending investigation An issue that needs additional investigation

Comments

@Bananeweizen
Copy link
Contributor

Describe the bug
There are 20+ tests failing on Windows because of using unsupported features.

  • the assertion conversion is implemented in shell scripts, and the test tries to run them
  • many path related tests do positive and negative testing for all kinds of assertions with a kind of variation of test cases, one or two being on symbolic links each time. As the javadoc of the symbolic link creation method states, that may fail on some systems because of missing privileges. That's the case for Windows.

I have a local change that disables these test methods (for the symbolic links) or test classes (for the conversion) on Windows and with a useful message. Would that be a reasonable fix as PR? The much harder alternative would be to recognize the IOException for the missing privileges in the tests, to verify their message and that it's Windows. I'm not a fan of such tests (and the message is localized for me, so not sure whether that even works).

@Bananeweizen Bananeweizen linked a pull request Sep 16, 2023 that will close this issue
@joel-costigliola
Copy link
Member

joel-costigliola commented Sep 17, 2023

Hi @Bananeweizen, we have windows builds as part of our CI and it works ok AFAIK.

I had a look at assertj-core/src/test/java/org/assertj/core/internal/paths/Paths_assertEndsWith_Test.java resutls and they pass:

2023-09-16T08:15:24.2101295Z [INFO] Running Paths assertEndsWith
2023-09-16T08:15:24.2246093Z [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.029 s -- in Paths assertEndsWith

Same for assertj-core/src/test/java/org/assertj/scripts/Convert_Junit_Assertions_To_Assertj_Test.java

2023-09-16T08:14:35.8067296Z [INFO] Running Convert JUnit assertions to AssertJ
2023-09-16T08:15:20.2271981Z [INFO] Tests run: 44, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 44.50 s -- in Convert JUnit assertions to AssertJ

So maybe it's something that is dependent on your machine (admin rights or something alike) ?

@Bananeweizen
Copy link
Contributor Author

But that would mean that you are probably running your CI tests in a non standard way, with admin rights or special settings for the virtual machine. Contributors with a normal setup (and not running as administrative user all the time locally) should see the same issue as me.

@scordio
Copy link
Member

scordio commented Sep 18, 2023

The files we manipulate should all be under a temporary directory created by JUnit, which in turn delegates to Files::createTempDirectory.

Are you sure you don't have any restrictions with your TMP folder?

But that would mean that you are probably running your CI tests in a non standard way, with admin rights or special settings for the virtual machine.

We just use the GitHub-hosted Windows runners, without any special settings.

Contributors with a normal setup (and not running as administrative user all the time locally) should see the same issue as me.

I should be able to get a similar setup. I'll have a look at it.

@scordio scordio self-assigned this Sep 18, 2023
@scordio scordio added the status: pending investigation An issue that needs additional investigation label Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: pending investigation An issue that needs additional investigation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants