Remove preferences and globals from tests#2768
Conversation
LinusDietz
left a comment
There was a problem hiding this comment.
Wow, that's cool progress. I have scrolled through the changes and have not found anything which I would say is of concern. Is there something I should have a closer look? otherwise I would say it can be merged.
lenhard
left a comment
There was a problem hiding this comment.
Wow! For a while, I have thought about how to best handle the preferences without finding a perfect solution, but this is certainly an advance. The preferences are not completely gone from logic, but if I see it correctly, then they are nicely encapsulated now. So +1 for merging.
Unfortunately, my time is a little bit too limited for a detailed review and I just scrolled over the code. That is why I do not merge directly now myself.
|
In general LGTM but please have a look at the fetcher test, there seems to be some NPEs occuring wich maybe related to this PR/the mocking |
|
Thanks for you feedback! Ups, I forgot to fix the fetcher tests. This is done now, except that the ISBN Chimbori fetcher seems to be totally broken (@koppor this is your baby, right?) |
|
I merge this now. Please open a new issue for the ISBN fetcher. |
* upstream/master: Reimplement date editor in JavaFX (#2781) Update CONTRIBUTING.md Add new author Update Checkstyle Version fix some more checkstyle warnings fix some more checkstyle warnings Fix Build failure, hopefully Spanish translation (#2773) Fixes #2766 If file is not found annotations might be null Fix language tests Remove preferences and globals from tests (#2768) Fix Unable to create Checker Fix checkstyle warnings New checkstyle rules regarding spacing Reimplement owner editior in JavaFX Reimplement url editior in JavaFX Reimplement journal editior in JavaFX New checkstyle rules regarding spacing # Conflicts: # src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java # src/main/java/org/jabref/migrations/FileLinksUpgradeWarning.java
|
This refs #110 (comment). |
This PR aims to make @lenhard happy again by introducing huge architectural constraints on our tests: they are not allowed to work with the preferences class anymore. As a positive side-effect also @koppor will be happy about this as tests now can no longer accidentally change or reset your preferences.
Changes in detail:
JabRefPreferences.getInstanceorGlobals.prefsby mocking away all preferences class (likeLayoutFormatterPreferences)KeyBindiningsandJournalAbbreviationsso that these classes don't operate on theJabRefPreferencesclass anymore. This required to hide some methods of the preferences in a new interfacePreferencesService.gradle localizationUpdate?