-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Parameterize tests in AuthorTest and AuthorListTest #14135
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Binaries
|
@YizhangCao seems to have saved the files with the wrong encoding. @Siedlerchr I see you closed the PR. Should he not fix this? |
|
We got a few low-quality and ai generated prs the last days. Looks like @Siedlerchr overlooked that this pr was from one of the students of your course, when he closed it. Sorry. Yes, please have the student update the branch with files in the correct encoding. |
|
Thank you @calixtus and @espertusnu for catching this encoding issue! I apologize for the problem. I've now fixed the file encoding to UTF-8. The issue occurred because I used PowerShell's redirection operator which saved the files as UTF-16 instead of UTF-8. After pushing I saw all tests are passed, right now I will fix [Source Code Tests / Checkstyle (pull_request)] and [Source Code Tests / Unit tests – jablib (pull_request) |
| @CsvSource({ | ||
| "'A O', 'A. O.'", | ||
| "'A-melia', 'A.-melia'", | ||
| "'AmeliA', 'AmeliA'", | ||
| "'Ameli A', 'Ameli A.'", | ||
| "'Ameli ', 'Ameli'", | ||
| "'Ameli AA', 'Ameli A. A.'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please refactor using the knowledge provided at https://devdocs.jabref.org/code-howtos/testing.html#use-paramterizedtests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @koppor and @calixtus for the review!
I've made the following fixes:
- Fixed all EMPTY_AUTHOR test cases in AuthorListTest.java to expect empty string ('') instead of null
- Reformatted all @CsvSource annotations in AuthorTest.java to have one test case per line for better readability, following the guidelines at https://devdocs.jabref.org/code-howtos/testing.html#use-paramterizedtests

Refs JabRef#676
This PR parameterizes tests in AuthorTest and AuthorListTest to reduce code duplication and improve test maintainability.
My classmates are also contributing to the same issue.
@espertusnu
Testing