Skip to content
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

Improve test code (one test per method, ParameterizedTests) #676

Open
4 tasks
koppor opened this issue Feb 13, 2024 · 2 comments
Open
4 tasks

Improve test code (one test per method, ParameterizedTests) #676

koppor opened this issue Feb 13, 2024 · 2 comments
Labels

Comments

@koppor
Copy link
Member

koppor commented Feb 13, 2024

(META: This issue is reserved for a university course. Please only work on it if it is part of your assignment)

Context

In unit tests, there should be one assertion per test. An anti-pattern is to include multiple, un-related assertions in one test method. For instance, this is the case at [AuthorListTest#fixAuthorLastNameFirstCommasOxfordComma]https://github.com/JabRef/jabref/blob/1ebe80224500556e3b5a2076f5d388671a59dd8d/src/test/java/org/jabref/model/entry/AuthorListTest.java#L338).

A good practice is to have Parameterized Tests instead of mass code duplication. For instance,

        assertEquals("Smith", AuthorList.fixAuthorLastNameOnlyCommas("John Smith", false));
        assertEquals("Smith", AuthorList.fixAuthorLastNameOnlyCommas("Smith, Jr, John", false));

can be replaced by

    @CsvSource(
            value = {
                    "Smith; John Smith;false",
                    "Smith; Smith, Jr, John;false",
            }, delimiter = ';')
    @ParameterizedTest
    public void fixAuthorLastNameOnlyCommasNew(String expected, String input, boolean oxfordComma) {
        assertEquals(expected, AuthorList.fixAuthorLastNameOnlyCommas(input, oxfordComma));
    }

One can also use @MethodSource if issues with @CsvSource are encountered.

The code

        assertEquals("", AuthorList.fixAuthorLastNameOnlyCommas("", false));

Cannot be covered using CsvSource, because the empty string in CsvSource is reduced to null by JUnit. Thus, keep that test separatere, and only test the non-empty string with CsvSource.

Tasks

  • Rewrite fixAuthorLastNameOnlyCommas using @CsvSource and @ParameterizedTest
  • Split AuthorListTest#getAuthor into three test methods
  • Rewrite AuthorListTest#removeStartAndEndBraces using @CsvSource and @ParameterizedTest
  • Try to split or rewrite other test methods in AuthorListTest
@Watson15
Copy link

Watson15 commented Mar 1, 2024

How would I test to make sure just my additional tests work instead of having to run ./gradlew test which runs all the tests?

@koppor
Copy link
Member Author

koppor commented Mar 1, 2024

On the guidelines to setup a workspace. At the bottom. There was an optional section. Please follow the steps there

https://devdocs.jabref.org/getting-into-the-code/guidelines-for-setting-up-a-local-workspace/intellij-12-build.html

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

No branches or pull requests

2 participants