Parameterize tests in AuthorTest and AuthorListTest #14135
Conversation
|
There seems to be something wrong with either github or your local configuration: |
|
@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.
Please refactor using the knowledge provided at https://devdocs.jabref.org/code-howtos/testing.html#use-paramterizedtests
There was a problem hiding this comment.
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
koppor
added
the
status: changes-required
koppor
left a comment
koppor
left a comment
There was a problem hiding this comment.
Please re-read the docs.
Use TEXT BLOCKS
| "MIXED_AUTHOR_AND_INSTITUTION_WITH_LATEX|The Banū Mūsā brothers and Böhm", | ||
| // LaTeX-free one institution with parenthesis at start | ||
| "ONE_INSTITUTION_WITH_STARTING_PARANTHESIS|Łukasz Michał" | ||
| }, delimiter = '|') |
There was a problem hiding this comment.
| is very unusal as delimiter - If you see it as Markdown table, please align the columns
Is ; available? (which is common).
There was a problem hiding this comment.
Thank you @koppor! I've updated all parameterized tests to use TEXT BLOCKS with delimiterString = "->" and aligned the columns for better readability as shown in the documentation. The tests now use # for comments and have properly aligned delimiters.
|
@YizhangCao It would be great if you could to the required changes asap. We want to reduce the number of open PRs |
|
Thanks for notice! I will do it as soon as possible! |
Thank you - and also please follow-up the automatic CI checks soon. |
github-actions
bot
removed
the
status: changes-required
| assertEquals("A. O.", Author.addDotIfAbbreviation("A.O.")); | ||
| assertEquals("A.-O.", Author.addDotIfAbbreviation("A-O")); | ||
| @ParameterizedTest | ||
| @CsvSource({ |
There was a problem hiding this comment.
But why there is no text block?
|
I got impatient and fixed for myself. @YizhangCao adapted only one class (and did not update |
|
Here the diff, what I updated: 23441b4...70c25e0 (there is one more minor thing, but that cannot be shown nicely, because I merged |
…ersions/org.glassfish.jersey.core-jersey-server-4.0.0 * upstream/main: (31 commits) Adds a 'regenerate' button to AI chat tab (#12191) (#14191) Chore(deps): Bump jablib/src/main/resources/csl-styles (#14323) Update dependency com.konghq:unirest-modules-gson to v4.6.0 (#14322) Convert fixed-value ComboBoxes to SearchableComboBox (#14083) (#14165) Add support for transliterated citation keys (#13893) Update dependency org.apache.maven.plugins:maven-jar-plugin to v3.5.0 (#14321) Add link to latest development version (#14320) Parameterize tests in AuthorTest and AuthorListTest (#14135) Add AGENTS.md and AI_USAGE_POLICY.md from p5.js (#14316) Fix Nullwarnings - A (#14116) Update io.github.darvil82:terminal-text-formatter from 2.2.0 to 2.3.0c (#14317) Streamline maven repositories (#14315) Fix paths to included Java sources Add forgotten .. Update dependency io.github.darvil82:terminal-text-formatter to v2.3.0c (#14314) Chore(deps): Bump io.github.classgraph:classgraph from 4.8.181 to 4.8.184 in /versions (#14304) Chore(deps): Bump com.fasterxml:aalto-xml in /versions (#14311) Chore(deps): Bump commons-io:commons-io in /versions (#14310) Chore(deps): Bump org.apache.maven.plugins:maven-surefire-plugin (#14298) Disable fetcher-gui-test (#14308) ...
* Parameterize tests in AuthorTest and AuthorListTest (issue JabRef#676) * Fix file encoding to UTF-8 * Fix EMPTY_AUTHOR test expectations and format CsvSource annotations * Use TEXT BLOCKS with -> delimiter for parameterized tests * Use CsvSource and TextBlock in AuthorTest * Use TextBlock * Reformat source * Use ";" instead of "->" * More text blocks * Convert to text block * Covnert one more * Use "," where possible * Use Markdown JavaDoc --------- Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
6 participants
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