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

Add more unit tests to three gui classes #7635

Closed
wants to merge 10 commits into from
Closed

Add more unit tests to three gui classes #7635

wants to merge 10 commits into from

Conversation

ningxie1991
Copy link
Contributor

This pull request contributes to issue #6207, to add more unit tests to the gui package, specifically using test doubles.

Tests added:

NewEntryActionTest
CopyMoreActionTest
ExportToClipboardActionTest

  • 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.

@Siedlerchr
Copy link
Member

thanks, looks already good :)

@ningxie1991 ningxie1991 reopened this Apr 16, 2021
ningxie1991 and others added 3 commits April 16, 2021 17:29
)

c363e8f Update catholic-biblical-association.csl (#5360)
7e383b8 Fix locators in PU Paris-Nanterre (#5376)
83cb249 Update methods-of-information-in-medicine.csl (#5370)
5b19db4 Update taylor-and-francis-council-of-science-editors-author-date.csl (#5371)
27d116b Update harvard-cite-them-right-no-et-al.csl (#5373)
e2ef408 Update ageing-and-society.csl (#5375)
16098e4 Update triangle.csl (#5369)
133d633 Create university-of-tasmania-simplified-author-date.csl (#5357)
b9ecf07 Update revue-archeologique.csl (#5366)
6152cce Update journal-of-consumer-research.csl (#5358)
93f6600 Create refugee-survey-quarterly.csl (#5354)
bfa3b6d Update canadian-biosystems-engineering.csl (#5362)
53e75ee update doi prefix (#5361)
5b3de98 Create archives-of-hand-and-microsurgery.csl (#5363)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: c363e8f

Co-authored-by: github actions <jabrefmail+webfeedback@gmail.com>

@BeforeEach
public void setUp() {
// if (Globals.prefs == null) {
Copy link
Contributor Author

@ningxie1991 ningxie1991 Apr 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Siedlerchr When I run this test, I get the error:
Cannot invoke "org.jabref.preferences.JabRefPreferences.getFilePreferences()" because "org.jabref.gui.Globals.prefs" is null

This is because line 84 in ExportToClipboardAction, it is calling:
Exporter defaultChoice = exporters.stream() .filter(exporter -> exporter.getName().equals(Globals.prefs.getImportExportPreferences().getLastExportExtension())) .findAny() .orElse(null);

I know we should not depend on Globals or alter the value of Globals.prefs in unit tests, but I was just trying if making Globals.prefs not null would solve the problem and it did. In this situation, how should we get arround this error the right way?

Thanks,
Ning

Copy link
Member

@Siedlerchr Siedlerchr Apr 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, as you cannot mock the Globals away, the only solution is to refactor the ExportToClipBoardAction, extend the constructor with an additional argument for PreferenceServices. As this is a bit more complicated, I just did this to show you how to do this if you encouter this again.

Edit// Can you enable https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork#enabling-repository-maintainer-permissions-on-existing-pull-requests
than I can push my changes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, you can merge this branch https://github.com/JabRef/jabref/tree/ningxie1991-a3-nx into yours

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Siedlerchr Thanks for the quick reply and help! I have merged your change into my branch. I will fix the test case now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Siedlerchr For some reason this pull requests suddenly includes 19 changed files. I think something happened when I merge your branch to mine. I replaced this pull request with a new one #7636. I am closing this pull request. Please do not merge this. Thanks.

Siedlerchr and others added 3 commits April 16, 2021 17:50
…ngxie1991-a3-nx

* 'a3-nx' of https://github.com/ningxie1991/jabref:
  fix checkstyle issue and comment out the use of Globals
  Modified the tests
  modified the tests
  organized imports
  Add JUnit test with mocks for CopyMoreAction
  Added two Junit tests using mocks for the gui
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants