Add unit test to four test classes #7651

BShaq · 2021-04-20T02:54:00Z

This pull request contributes to issue #6207, which is to add more unit tests to the project.

Tests added:

DuplicateSearchTest
ManageKeywordsViewModelTest
GroupDiffTest

Tests extended:

GroupTreeNodeTest

Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

BShaq · 2021-04-20T02:58:36Z

src/test/java/org/jabref/gui/duplicationFinder/DuplicateSearchTest.java

+        when(undoManager.addEdit(mock(NamedCompound.class))).thenReturn(true);
+
+        duplicateSearch.execute();
+        verify(dialogService, times(1)).notify(Localization.lang("Searching for duplicates..."));


Hi :)
I saw on the discussion #7636 that you would prefer using any() over a localized argument for the notify() method. The rationale for using this localized argument was to verify if dialogService shows the correct message for each condition. As @ningxie1991 pointed out, would the dialogService somehow loose its purpose when using any() as the argument?

Moreover, in this case, if I use the localized argument, both very() methods work, but by using any() as argument I receive the feedback that the dialogService got invoked only once. So, what did I miss on here?

It's okay to use the correct localization argument. I think we thought that it just verifies the numbers of calls, but not the content

Siedlerchr

Some minor improvements, but overall looks good to me so far

src/test/java/org/jabref/gui/edit/ManageKeywordsViewModelTest.java

src/test/java/org/jabref/logic/bibtex/comparator/GroupDiffTest.java

koppor

Small nitpick comments to have the tests indepdenent of the user's configuration.

I am not sure about the benefit of these tests. It tests some GUI functionality, but not some logic functionality. Therefore, I added to #6207 that we do not want to have any GUI tests added as they are not simple at all.

src/test/java/org/jabref/gui/duplicationFinder/DuplicateSearchTest.java

src/test/java/org/jabref/gui/edit/ManageKeywordsViewModelTest.java

Made private constructor package-private to be able to create a GroupDiff instance for testing. The test fails when comparing the Optional<GroupDiff> objects directly due to the different object IDs, despite the fact that their fields should be identical. Therefore, I decided to compare the fields of the objects to check if they are equal.

Moved setup of Globals.prefs into the setupGlobals() method

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

BShaq · 2021-05-15T22:54:25Z

Hi,
I could finally apply your comments.

koppor

There are failing GUI tests which should be related to your work, isn't it?

calixtus · 2021-05-24T19:24:18Z

Hi @BShaq , we were just looking in all of our open PRs and saw yours. Did you have some time to look into the failing test already?

BShaq · 2021-05-24T20:44:04Z

Hi @calixtus & @koppor . I'm very sorry that it takes so long to reply to all of your comments, it has been a busy few weeks for me..
I'm just looking into it now and realize that when I run it locally, the test does not fail at all as you can see on the attached screenshot:

However, I also see some warnings and an error right below: 'Messages are not initialized before accessing key: Searching for duplicates...'
Is it possible that this is the reason that the build fails?

koppor · 2021-06-29T21:45:24Z

Hi @calixtus & @koppor . I'm very sorry that it takes so long to reply to all of your comments, it has been a busy few weeks for me..

Same for us 😇

I'm just looking into it now and realize that when I run it locally,

May I ask whether you ran all tests?

I get the following output:

org.jabref.architecture.TestArchitectureTests

  Test [1] org.jabref.preferences.JabRefPreferences FAILED

  org.opentest4j.AssertionFailedError: The following classes are not allowed to depend on org.jabref.preferences.JabRefPreferences ==> expected: <[]> but was: <[src/test/java/org/jabref/gui/duplicationFinder/DuplicateSearchTest.java]>

I fixed that in 29718c5 (#7651).

However, I also see some warnings and an error right below: 'Messages are not initialized before accessing key: Searching for duplicates...'
Is it possible that this is the reason that the build fails?

No, the warning is OK

Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>

* upstream/main: (26 commits) Add unit test to four test classes (#7651) Fix IEEE test (#7852) New Crowdin updates (#7859) Fix markdown syntax of ADRs add missing l10n (#7857) New Crowdin updates (#7847) Bump mockito-core from 3.11.1 to 3.11.2 (#7856) Bump checkstyle from 8.43 to 8.44 (#7855) Fix for issue #4652: Add Find Unlinked Files Filter based on Date (#7846) Fix for entering a backslash in the custom entry preview dialog (#7851) Fixed INSPIREFetcherTest Fixed TitleFetcherTest Ignore baeldung.com and tldrlegal.com from out link checks New Crowdin updates (#7845) New Crowdin updates (#7843) Refactoring and addition of unit tests (#7597) CLI option to write XMP metadata to pdfs (#7814) Add query validation for web search (#7809) change eclipse default output dir (#7842) Bump lucene-queryparser from 8.8.2 to 8.9.0 (#7835) ...

…kflow-for-slr-search * upstream/main: (31 commits) New translations JabRef_en.properties (German) (#7868) Fix test "higherTrustLevelWins()" (#7866) Change WM_CLASS to jabref (#7858) [Bot] Update CSL styles (#7865) Add unit test to four test classes (#7651) Fix IEEE test (#7852) New Crowdin updates (#7859) Fix markdown syntax of ADRs add missing l10n (#7857) New Crowdin updates (#7847) Bump mockito-core from 3.11.1 to 3.11.2 (#7856) Bump checkstyle from 8.43 to 8.44 (#7855) Fix for issue #4652: Add Find Unlinked Files Filter based on Date (#7846) Fix for entering a backslash in the custom entry preview dialog (#7851) Fixed INSPIREFetcherTest Fixed TitleFetcherTest Ignore baeldung.com and tldrlegal.com from out link checks New Crowdin updates (#7845) New Crowdin updates (#7843) Refactoring and addition of unit tests (#7597) ... # Conflicts: # src/main/resources/l10n/JabRef_en.properties

BShaq added 7 commits April 19, 2021 17:35

Added unit tests with mocks for DuplicationSearch

c67512d

Added unit tests with mocks for ManageKeywordsViewModel

35c486b

Added unit tests with mocks for GroupDiff

54dbb0d

organized imports in DuplicateSearch

d74f0a7

Merge remote-tracking branch 'upstream/main' into a3-bs

6953da2

Added unit test to GroupTreeNodeTest.java

54a6e3b

remove outcommeted code

16e8b59

BShaq commented Apr 20, 2021

View reviewed changes

BShaq added 2 commits April 23, 2021 11:09

fixed checkstyle

b71be0d

Merge remote-tracking branch 'upstream/main' into a3-bs

4b7f78e

Siedlerchr requested changes Apr 24, 2021

View reviewed changes

BShaq added 3 commits April 26, 2021 21:40

switched to comparing content of lists in ManageKeywordsViewModelTest

81e79ee

removed outcommented code

3ecb4f8

Merge remote-tracking branch 'upstream/main' into a3-bs

97b8217

koppor requested changes May 1, 2021

View reviewed changes

src/test/java/org/jabref/gui/duplicationFinder/DuplicateSearchTest.java Outdated Show resolved Hide resolved

src/test/java/org/jabref/gui/edit/ManageKeywordsViewModelTest.java Outdated Show resolved Hide resolved

Merge remote-tracking branch 'upstream/main' into a3-bs

bd1d218

koppor added the status: changes required Pull requests that are not yet complete label May 2, 2021

BShaq and others added 6 commits May 16, 2021 00:21

created BeforeAll method

a867e3a

Moved setup of Globals.prefs into the setupGlobals() method

fixed checkstyle issue

b8c996e

made entries creation more concise

42fc053

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

Merge remote-tracking branch 'upstream/main' into a3-bs

6cd9986

Merge branch 'a3-bs' of https://github.com/ningxie1991/jabref into a3-bs

cd37a64

Fix checkstyle

948f6c0

koppor requested changes May 17, 2021

View reviewed changes

Siedlerchr closed this May 24, 2021

Siedlerchr reopened this May 24, 2021

koppor changed the base branch from main to follow-up-to-7651 June 29, 2021 21:17

Merge branch 'main' into ningxie1991-a3-bs

17c5b31

koppor changed the base branch from follow-up-to-7651 to main June 29, 2021 21:19

Remove Globals dependency

29718c5

Fix Codacy

d4d8170

Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>

koppor merged commit a01754c into JabRef:main Jun 29, 2021

koppor mentioned this pull request Jul 4, 2021

Remove DuplicateSearchTest #7875

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unit test to four test classes #7651

Add unit test to four test classes #7651

BShaq commented Apr 20, 2021 •

edited

Loading

BShaq Apr 20, 2021

Siedlerchr Apr 24, 2021

Siedlerchr left a comment

koppor left a comment

BShaq commented May 15, 2021

koppor left a comment

calixtus commented May 24, 2021

BShaq commented May 24, 2021

koppor commented Jun 29, 2021 •

edited

Loading

Add unit test to four test classes #7651

Add unit test to four test classes #7651

Conversation

BShaq commented Apr 20, 2021 • edited Loading

BShaq Apr 20, 2021

Choose a reason for hiding this comment

Siedlerchr Apr 24, 2021

Choose a reason for hiding this comment

Siedlerchr left a comment

Choose a reason for hiding this comment

koppor left a comment

Choose a reason for hiding this comment

BShaq commented May 15, 2021

koppor left a comment

Choose a reason for hiding this comment

calixtus commented May 24, 2021

BShaq commented May 24, 2021

koppor commented Jun 29, 2021 • edited Loading

BShaq commented Apr 20, 2021 •

edited

Loading

koppor commented Jun 29, 2021 •

edited

Loading