Skip to content

Disable test not working on Windows #12869

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Disable test not working on Windows #12869

wants to merge 5 commits into from

Conversation

koppor
Copy link
Member

@koppor koppor commented Mar 31, 2025

With this PR, all tests in :test should work on Window

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • [.] Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • [.] Tests created for changes (if applicable)
  • [.] Manually tested changed features in running JabRef (always required)
  • [.] Screenshots added in PR description (if change is visible to the user)
  • [.] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • [.] Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Comment on lines +50 to +52
void tearDown() {
this.indexer.closeAndWait();
}
Copy link

Choose a reason for hiding this comment

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

The tearDown method is unnecessary when using @tempdir, as JUnit automatically handles cleanup of temporary directories. This violates the instruction to avoid redundant cleanup.

Copy link
Member Author

Choose a reason for hiding this comment

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

No. The indexer locks the directory!

@koppor
Copy link
Member Author

koppor commented Apr 1, 2025

Together with #12868, maybe we can get rid of this one?

image

@koppor koppor requested review from Siedlerchr and subhramit April 1, 2025 21:06
@subhramit
Copy link
Member

Together with #12868, maybe we can get rid of this one?

Yes, there is a lot of overlap right now between the bot texts and FAQ page.

Copy link

trag-bot bot commented Apr 4, 2025

@trag-bot didn't find any issues in the code! ✅✨

Copy link
Contributor

github-actions bot commented Apr 4, 2025

The build of this PR is available at https://builds.jabref.org/pull/12869/merge.

@subhramit
Copy link
Member

I still get 31 failing tests
image
image

@Siedlerchr
Copy link
Member

What is the status here?

@koppor
Copy link
Member Author

koppor commented Apr 10, 2025

What is the status here?

Someone needs to go through the failing tests and decide whether they should be fixed or if it can be disabled.

I plan to do it as soon as I find time.

@koppor koppor marked this pull request as draft April 10, 2025 16:12
auto-merge was automatically disabled April 10, 2025 16:12

Pull request was converted to draft

@koppor
Copy link
Member Author

koppor commented Apr 17, 2025

Side track: These tests take really long - I don't understand why

grafik

@koppor
Copy link
Member Author

koppor commented Apr 17, 2025

I used gradle (!) to run the tests - result:

grafik

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

Successfully merging this pull request may close these issues.

3 participants