Refactor StringUtilTest to use parameterized tests #14126

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.

Sign up for GitHub

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

Jump to bottom

Merged

koppor merged 14 commits into JabRef:main from Shanaya-1981:contribution-to-issue-676

Nov 13, 2025

Contributor

Shanaya-1981 commented Oct 21, 2025 •

edited

Loading

Refs #676

This PR refactors StringUtilTest to use parameterized tests (@CsvSource and @MethodSource) following the one-assertion-per-test guideline.

Note: My classmates are also contributing to this issue.

How to test:

./gradlew :jablib:test --tests org.jabref.model.strings.StringUtilTest

Mandatory checks

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 described the change in CHANGELOG.md in a way that is understandable for the average user (if change is visible to the user)
I checked the user 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 updating file(s) in https://github.com/JabRef/user-documentation/tree/main/en.

Shanaya-1981 added 5 commits

October 14, 2025 15:11


          Parameterize tests (issue JabRef#676)

e56ad41


          Remove unnecessary quotes in CSV tests

950ed00


          Apply IntelliJ auto-format

16ff6ec


          Fix indentation in StringUtilTest

b02baca


          Merge remote-tracking branch 'origin/main' into contribution-to-issue…

a9e6b8a

…-676

Member

Siedlerchr commented Oct 28, 2025 •

edited

Loading

In the meantime another PR was merged, which moved the StringUtil classes to tbe logic package . Please update your branch and adapt to the changes and follow the code style

Siedlerchr added dev: testing status: changes-required labels

Shanaya-1981 added 3 commits

October 30, 2025 15:27


          Fix code formatting in StringUtilTest

9c7cb79


          Merge remote-tracking branch 'upstream/main' into contribution-to-iss…

3990fab

…ue-676


          Remove csl-styles/locales file

c7dd338

koppor requested changes

View reviewed changes

jablib/src/test/java/org/jabref/model/strings/StringUtilTest.java Outdated Show resolved Hide resolved


          Use text blocks and fix formatting

a01f57c

Member

Siedlerchr commented Nov 3, 2025

Please check https://devdocs.jabref.org/code-howtos/faq.html#submodules

Shanaya-1981 added 2 commits

November 6, 2025 15:25


          Revert "Remove csl-styles/locales file"

c4ab41b

This reverts commit c7dd338.


          Fix format issue in textblock test

afddf2a

koppor reviewed

View reviewed changes

jablib/src/test/java/org/jabref/logic/util/strings/StringUtilTest.java Outdated Show resolved Hide resolved

koppor reviewed

View reviewed changes

jablib/src/test/java/org/jabref/logic/util/strings/StringUtilTest.java Outdated Show resolved Hide resolved

Shanaya-1981 added 2 commits

November 10, 2025 18:51


          Refactor tests to use text block format

e49e68a


          Fix text block indentation

87a2f01

koppor requested changes

View reviewed changes

Member

koppor left a comment

Much, much better!

I have some small things which would make the tests even better. Maybe, you can do that, too?

jablib/src/test/java/org/jabref/logic/util/strings/StringUtilTest.java Outdated

    
                      // assertArrayEquals(stringArray2res, StringUtil.decodeStringDoubleArray(encStringArray2));

                      assertArrayEquals(new String[][] {{"a", ":b"}, {"c;", "d"}}, StringUtil.decodeStringDoubleArray("a:\\:b;c\\;:d"));

                  @ParameterizedTest

                  @MethodSource("provideDecodeStringDoubleArrayData")

Member

koppor Nov 11, 2025

Suggested change

      
                @MethodSource("provideDecodeStringDoubleArrayData")
          
                @MethodSource

jablib/src/test/java/org/jabref/logic/util/strings/StringUtilTest.java Outdated

    
                      assertFalse(StringUtil.isInCurlyBrackets("}"));

                      assertFalse(StringUtil.isInCurlyBrackets("a{}a"));

                      assertFalse(StringUtil.isInCurlyBrackets("{\\AA}sa {\\AA}Stor{\\aa}"));

                  static Stream<Arguments> provideDecodeStringDoubleArrayData() {

Member

koppor Nov 11, 2025

Name the method the same as the "calling" method.

Suggested change

      
                static Stream<Arguments> provideDecodeStringDoubleArrayData() {
          
                static Stream<Arguments> decodeStringDoubleArray() {

jablib/src/test/java/org/jabref/logic/util/strings/StringUtilTest.java

Comment on lines 228 to 242

    
                  @ParameterizedTest

                  @CsvSource(textBlock = """

                          false,

                          true, {}

                          true, {a}

                          true, '{a{a}}'

                          true, '{{\\AA}sa {\\AA}Stor{\\aa}}'

                          false, {

                          false, }

                          false, a{}a

                          false, '{\\AA}sa {\\AA}Stor{\\aa}'

                          """)

                  void isInCurlyBrackets(boolean expected, String input) {

                      assertEquals(expected, StringUtil.isInCurlyBrackets(input));

                  }

Member

koppor Nov 11, 2025

Split into two tests

isInCurlyBrackets

isNotInCurlyBrackets

jablib/src/test/java/org/jabref/logic/util/strings/StringUtilTest.java

Comment on lines 244 to 255

    
                  @ParameterizedTest

                  @CsvSource(textBlock = """

                          false,

                          true, []

                          true, [a]

                          false, [

                          false, ]

                          false, a[]a

                          """)

                  void isInSquareBrackets(boolean expected, String input) {

                      assertEquals(expected, StringUtil.isInSquareBrackets(input));

                  }

Member

koppor Nov 11, 2025

See above (split into two tests)

jablib/src/test/java/org/jabref/logic/util/strings/StringUtilTest.java

Comment on lines 234 to 267

    
                  @Test

                  void intValueOfSingleDigit() {

                      assertEquals(1, StringUtil.intValueOf("1"));

                      assertEquals(2, StringUtil.intValueOf("2"));

                      assertEquals(8, StringUtil.intValueOf("8"));

                  @ParameterizedTest

                  @CsvSource(textBlock = """

                          false,

                          true, ""

                          true, "a"

                          false, "

                          false, a""a

                          """)

                  void isInCitationMarks(boolean expected, String input) {

                      assertEquals(expected, StringUtil.isInCitationMarks(input));

                  }

Member

koppor Nov 11, 2025

See above (split into two tests)


          Address review comments

d567c78

github-actions bot removed the status: changes-required label

koppor approved these changes

View reviewed changes

Member

koppor left a comment

I fixed for myself to keep things going. Now, its ready for merge.

koppor enabled auto-merge

November 13, 2025 08:45

koppor added this pull request to the merge queue

Merged via the queue into JabRef:main with commit d6fda1d

45 checks passed

koppor mentioned this pull request

Parameterize tests in logic.formatter.bibtexfields #14258

Open

Siedlerchr added a commit that referenced this pull request


          Merge remote-tracking branch 'upstream/main' into jackson3

5aee419

* upstream/main: (30 commits)
  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)
  Chore(deps): Bump com.dlsc.gemsfx:gemsfx in /versions (#14307)
  No labels for dependeabot updates
  Fix fallback window height from 786 to 768 (#14295)
  Chore(deps): Bump com.fasterxml.jackson.dataformat:jackson-dataformat-yaml (#14306)
  Chore(deps): Bump org.apache.maven.plugins:maven-compiler-plugin (#14302)
  Chore(deps): Bump org.apache.maven.plugins:maven-deploy-plugin (#14300)
  Chore(deps): Bump org.apache.maven.plugins:maven-clean-plugin (#14299)
  Chore(deps): Bump jablib/src/main/resources/csl-styles (#14296)
  Chore(deps): Bump com.vanniktech.maven.publish in /jablib (#14303)
  Chore(deps): Bump org.apache.maven.plugins:maven-project-info-reports-plugin (#14297)
  Update all dependencies (#14301)
  Prepare maven3 example project (#14294)
  Refactor StringUtilTest to use parameterized tests (#14126)
  Try to fix download of pr_number
  Fix workflow names
  ...

merlinymy pushed a commit to merlinymy/jabref that referenced this pull request


          Refactor StringUtilTest to use parameterized tests (JabRef#14126)

7e266b2

* Parameterize tests (issue JabRef#676)

* Remove unnecessary quotes in CSV tests

* Apply IntelliJ auto-format

* Fix indentation in StringUtilTest

* Fix code formatting in StringUtilTest

* Remove csl-styles/locales file

* Use text blocks and fix formatting

* Revert "Remove csl-styles/locales file"

This reverts commit c7dd338.

* Fix format issue in textblock test

* Refactor tests to use text block format

* Fix text block indentation

* Address review comments

---------

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>

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

Labels