Fix failed test on Windows

[pluto-han]

Related issues and pull requests

Closes #15537

PR Description

Add linebreak unifier to method in RelatedWorkInserterTest.

Steps to test

On Windows, run test insertMatchedRelatedWorkAppendsToExistingUserSpecificCommentField in RelatedWorkInserterTest, it should pass now.

Checklist

• I own the copyright of the code submitted and I license it under the MIT license
• I manually tested my changes in running JabRef (always required)
• [/] I added JUnit tests for changes (if applicable)
• [/] I added screenshots in the PR description (if change is visible to the user)
• [/] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
• [/] I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
• [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Fix Windows test failure with unified line breaks

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Replace OS-specific newline constant with unified line break handling
• Use StringUtil.unifyLineBreaks() for cross-platform test compatibility
• Fix Windows test failure in RelatedWorkInserterTest

Diagram

flowchart LR
  A["OS.NEWLINE constant"] -- "replace with" --> B["StringUtil.unifyLineBreaks()"]
  B -- "ensures" --> C["Cross-platform compatibility"]
  C -- "fixes" --> D["Windows test failure"]

Loading

File Changes

1. jablib/src/test/java/org/jabref/logic/relatedwork/RelatedWorkInserterTest.java 🐞 Bug fix +2/-2

Replace OS newline constant with unified line breaks

• Changed import from OS to StringUtil for line break handling
• Replaced OS.NEWLINE constant with StringUtil.unifyLineBreaks() method call
• Added .stripTrailing() to remove trailing whitespace before unification
• Explicitly use \n character for consistent line break representation

jablib/src/test/java/org/jabref/logic/relatedwork/RelatedWorkInserterTest.java

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

ⓘ The new review experience is currently in Beta. Learn more

[koppor]

Not sure why double (!) changes

• Use \n instead of OS.NEWLINE
• Unify linebreaks at tests

I think, the first one is enough to do.

If BOTH is needed, there should be a comment

[subhramit]

I think the confusion arises from a prior discussion #15316 (comment)

[pluto-han]

Yes. In #15316 (comment), my thought is that we should first normalize the linebreak at the end of the sentence, before adding new linebreaks, so I use unifyLineBreaks.

In this test, I think we don't need unifyLineBreaks because the sentences contain no linebreaks. But I am not sure if we should also remove unifyLineBreaks in RelatedWorkInserter.

[calixtus]

Move fast, break things, and fix them later.

[subhramit]

You ticked that you modified CHANGELOG.md, but no new entry was found there.

If you made changes that are visible to the user, please add a brief description along with the issue number to the CHANGELOG.md file. If you did not, please replace the cross ([x]) by a slash ([/]) to indicate that no CHANGELOG.md entry is necessary. More details can be found in our Developer Documentation about the changelog.

false alarm

[subhramit]

We don't normalize in tests, hence the commit 1337303

Edit (for addition to original context) - it might be the case that JavaFX normalizes to \n as pointed out in #15316 (comment), but there is no harm in keeping the backend logic more general/independent.

Issue tracked for follow up.

[pluto-han]

Sorry for being inactive, just finished the last exam.

Should I make a follow-up PR to fix problems in #15538 (comment)?

[subhramit]

We don't normalize in tests, hence the commit 1337303

Edit (for addition to original context) - it might be the case that JavaFX normalizes to \n as pointed out in #15316 (comment), but there is no harm in keeping the backend logic more general/independent.

@pluto-han I think the above^ justifies this PR as the fix? Do you have anything else in mind?

[pluto-han]

I think the above^ justifies this PR as the fix?

Yep I think then we can keep this code.