Cleanup FindFile and asssociated tests #1596

Siedlerchr · 2016-07-16T17:20:39Z

Cleaned up the findFiles method
Reworked tests, added some new Tests
Tests should now work in Linux, too

Tests created for changes

Siedlerchr · 2016-07-16T19:56:29Z

I have no idea why the test fails, however, it seems to be a Linux related problem.
Could anyone please test my code on Linux?

oscargus · 2016-07-17T13:59:07Z

src/main/java/net/sf/jabref/logic/util/io/FileFinder.java

-        if (children == null) {
-            return result; // No permission?
-        }
+        Objects.requireNonNull(directories, "Directories must not null!");


oscargus · 2016-07-17T14:06:21Z

No idea why the tests fail. Interesting that the NPE test is failing as well...

Siedlerchr · 2016-07-17T14:42:43Z

@oscargus Yeah, that bothered me, too. I tested on a Linux VM and it only fails there.
I will try to debug further.

oscargus · 2016-07-18T11:52:47Z

src/test/java/net/sf/jabref/logic/util/io/UtilFindFileTest.java

+        Assert.assertTrue(results.get(entry)
+                .contains(rootPath.resolve(Paths.get("graphicsDir", "subDir", "HipKro03test.jpg")).toFile()));
+        Assert.assertFalse(results.get(entry)
+                .contains(rootPath.resolve(Paths.get("graphicsDir", "subDir", "HipKro03test.png")).toFile()));


Could it be that it says png here but pdf in extensions? (Not sure why it would work on Windows though...)

Oh, is said assertFalse. That explains things. :-)

The problem is that under linux the first AssertEquals fails. because it says size is 1 but excepted 2.

@oscargus I think I found the error! Case Sensitivy. Windows is case insenstive, but Linux not!
graphicsDirvs graphicsdir

Seems reasonable! Well hunted!

Siedlerchr · 2016-07-18T12:29:04Z

I will debug it tomorrow at university. I have absolutely no idea why it fails. All stuff is done in the BaseClass in the Before Method.

Siedlerchr · 2016-07-18T17:58:05Z

Okay, only the Exeption test did not work...

simonharrer · 2016-07-19T12:21:47Z

src/test/java/net/sf/jabref/FileBasedTestCase.java

-        FileBasedTestHelper.deleteRecursive(root);
+
+        try {
+            for (Path p : Files.walk(rootDir).sorted((a, b) -> b.compareTo(a)). // reverse; files before dirs


Files.walk creates a stream, which must be closed using try-with-resources, otherwise, this may affect another test as it will not free the directory resource.

simonharrer · 2016-07-21T12:14:08Z

Do you need any help here?

Siedlerchr · 2016-07-21T13:15:51Z

@tobiasdiez Yes, I still have no idea why the test is failing only on the CI Server. Both under win and linux all tests are passing. Do you have a clue what the reason can be?

simonharrer · 2016-07-21T13:48:41Z

You could try to get more information from Gradle about the failing test. Maybe run it with --stacktrace in the travis configuration, and have a look at the gradle test configuration so that the full stack trace is shown. This could help to find the cause.

stefan-kolb · 2016-07-21T14:47:49Z

src/main/java/net/sf/jabref/logic/util/io/FileFinder.java


-        return result;
-    }
+    public static Set<File> findFiles(Collection<String> extensions, Collection<File> directories) {


Maybe just use List here instead of Collection?

tobiasdiez · 2016-07-21T16:50:03Z

Just a quick guess: maybe the @Before method from the base class is not called and thus the root directory is null, which gives the NPE. You might try to call the initialization method explicitly, see http://stackoverflow.com/a/2132852/873661

Siedlerchr · 2016-07-21T17:10:43Z

@tobiasdiez Thanks . I already thought that it may be related to the sublassing.
I will investigate this option

Use Lazy Stream to find files Add some more test, reworked test Tests should now work on Linux, too Completey rework File and dir creating using nio methods Fix case typo which was reason for linux bug Move sublass test methods into baseclass hopefully fixes CI build

Set NPE Test to ignore, as it fails only on CI server

Hopefully fixes problem for CI Server...

Siedlerchr · 2016-07-21T19:04:40Z

Okay, not it only fails at the tearDownmethod, when restoring the prefs.
@oscargus Could you please have a look, as you recently changed the Globals.pref stuff?

oscargus · 2016-07-22T16:58:04Z

Well, I didn't actually change anything there. Only encapsulated the use of it in a number of places, so shouldn't affect this.

* master: Added LayoutFormatterPreferences (and related files) (JabRef#1608) [WIP] Create new fetcher infrastructure (JabRef#1594) Set user agent to fix 403 status error More fields added to FieldName (JabRef#1614)

Mock Prefs

Siedlerchr · 2016-07-22T20:03:30Z

Finally all tests green 🍰

oscargus · 2016-07-22T21:55:17Z

👍

simonharrer · 2016-07-23T09:48:26Z

Hm, what about the still marked @Ignore test?

tobiasdiez · 2016-07-23T09:59:47Z

src/test/java/net/sf/jabref/logic/util/io/FileBasedTestCase.java

+        BibDatabase database = BibtexTestData.getBibtexDatabase();
+        entry = database.getEntries().iterator().next();
+
+        rootDir = Files.createTempDirectory("UtilFindFileTest");


I think the proper way to handle temporary folders in tests is via
@Rule public TemporaryFolder temporaryFolder = new TemporaryFolder();,
which also takes care of deleting the folder after every test.

use TemporaryFolder use Collections.emptyset for expexted

* master: Always use https for help files (JabRef#1615) Resolves JabRef#1613 Use Jabref default icon for uninstaller for now (JabRef#1616) Added more fields and fixed some issues (JabRef#1617)

* master: More field names and a method (JabRef#1627) Moved, removed, and used String constants (JabRef#1618) Updated preview entries (JabRef#1606) Improved LaTeX to Unicode/HTML formatters to output more sensible values for unknown commands (JabRef#1622) Fixed JabRef#636 by using DOICheck and DOIStrip in export filters (JabRef#1624) Converted a few getField to getFieldOptional (JabRef#1625) Added DateFormatter to LayoutEntry so that it actually works... (JabRef#1619) Implemented JabRef#1345 - cleanup ISSN (JabRef#1590)

Siedlerchr · 2016-07-25T18:12:36Z

@simonharrer The formerly ignored NPE test works now, too.
If no more things, then I would squash and merge.

simonharrer · 2016-07-25T18:35:11Z

Go ahead. Nice job! :) Maybe we can reduce the number of ignored tests over time.

* master: define xjc input/ouput dir (subsequent builds will be faster) (JabRef#1628) Consistent file name casing (and other localization improvements) (JabRef#1629) Builds are now stored via build-upload.jabref.org Some enhancements and cleanups related to dates (JabRef#1575) changes should be tested manually Cleanup FindFile and asssociated tests (JabRef#1596) More field names and a method (JabRef#1627) Moved, removed, and used String constants (JabRef#1618) Updated preview entries (JabRef#1606) Improved LaTeX to Unicode/HTML formatters to output more sensible values for unknown commands (JabRef#1622) Fixed JabRef#636 by using DOICheck and DOIStrip in export filters (JabRef#1624) Converted a few getField to getFieldOptional (JabRef#1625)

@comment

Move event (JabRef#1601) * Move event package to model Update dependencies: postgres 9.4.1208 -> 9.4.1209 and wiremock from 2.1.6 to 2.1.7 Added ISBN integrity checker (JabRef#1586) Added ISBN integrity checker * Extracted ISBN class Reenable errorprone (see http://errorprone.info/) Extend the OpenConsoleFeature (JabRef#1582) * Extend the OpenConsoleFeature by selection of custom terminal emulator. - Add radio selection to the AdvancedTab - Add new JabRefPreferences - Add file check and execution commands - Add localization keys * Fix localization key. * Move console selection to ExternalTab.java * Change localization entry. * Add command executor. * Fix placeholder replacement. * Fix codacy. * Update localization key. * Remove "Specify terminal emulator" option. Add GUI outputs. * Set default command for Windows. Fix localization entries. * Remove empty lines in language files. * Use lambda expressions insead of ActionListeners * Refactoring. * Update CHANGELOG.md. * Small refactorings. Move preferences (JabRef#1604) * Move preferences-related classes into separate package * Rename JabRefPreferencesFilterDialog -> PreferencesFilterDialog and move it to gui * Fix checkstyle warning Set user agent to fix 403 status error Replace getField with getFieldOptional in all of the tests and in som… (JabRef#1591) * Replace getField with getFieldOptional in all of the tests and in some more code * Some more conversions Added filter to not show selected integrity checks (JabRef#1588) * Added filter to not show selected integrity checks * Removed unused variable Some Globals.prefs injection in logic and model (JabRef#1593) * Some Globals.prefs injection in logic and model * Some more conversions and some fixes * More injections * Even more injections * Yes, even more injections * Indeed, even more injections * Probably the last injections for now * Removed unrequired dependency and fixed issue * Dropped support for selecting sub/super to equations * Added preference classes for LatexFieldFormatter and FieldContentParser * Removed some left over code * Added JournalAbbreviationPreferences * Encapsulated LatexFieldFormatterPreferences in SavePreferences * Changed getShortDescription to accept boolean argument * Removed Globals.prefs from tests and removed unused imports * Unused import * Unused import Rewrite MedlineImporter with JAXB and add nbib fields (JabRef#1479) Add test in BibEntryWriterTest for type change When clicking on a tab, the first field now has the focus (JabRef#988) * the first Field does now have focus when clicking on a tab in the entry editor * Make first field focused when selecting a tab in entry editor The field list gets the focus as soon as it is focused (JabRef#1541) Test CustomImporter (JabRef#1501) * Test CustomImporter Fixes imports Added model.entry.FieldName that contains field name constants (JabRef#1602) * Added model.entry.FieldName that contains field name constants * More constants * Renamed and added more constants * Some more fields and cleanups * Removed MedlineHandler left from merge conflicts More fields added to FieldName (JabRef#1614) * More fields added to FieldName * Some Medline fixes [WIP] Create new fetcher infrastructure (JabRef#1594) * Introduce new Fetcher interfaces * Refactor arXiv fulltext fetcher * Add query based arXiv fetcher * Reformat code * Add a few tests for the arxiv parser * Make new arXiv fetcher available * Fix small problems related to files * Fix tests * Rename interface methods * Add changelog entry * Mark old EntryFetcher interface as deprecated * Move fetcher to importer \ fetcher * Move HelpFile from gui.help to logic.help * Rename fetchers * Rename FulltextFinder * Optimize imports * Fix failing test * Ignore failing test Added LayoutFormatterPreferences (and related files) (JabRef#1608) * Added LayoutFormatterPreferences (and related files) * Rebased * Included JournalAbbreviationLoader in LayoutPreferences Added more fields and fixed some issues (JabRef#1617) Resolves JabRef#1613 Use Jabref default icon for uninstaller for now (JabRef#1616) Always use https for help files (JabRef#1615) Implemented JabRef#1345 - cleanup ISSN (JabRef#1590) * Implemented JabRef#1345 - cleanup ISSN * Fixed comments * Extracted ISSN class * Added tests for ISSN and ISBN Added DateFormatter to LayoutEntry so that it actually works... (JabRef#1619) Converted a few getField to getFieldOptional (JabRef#1625) * Converted a few getField to getFieldOptional Fixed JabRef#636 by using DOICheck and DOIStrip in export filters (JabRef#1624) Improved LaTeX to Unicode/HTML formatters to output more sensible values for unknown commands (JabRef#1622) Updated preview entries (JabRef#1606) * Updated preview entries, which return new entry Moved, removed, and used String constants (JabRef#1618) * Moved, removed, and used String constants * Some more fixes * Moved NEWLINE, made FILE_SEPARATOR public and used it * Moved NEWLINE and FILE_SEPARATOR to OS * Moved ENCODING_PREFIX and SIGNATURE * Corrected Globals in a few comments... * Apparently the localization tests find commented out text... More field names and a method (JabRef#1627) * Introduced FieldName in ArXiV * Some more field names * More field names Cleanup FindFile and asssociated tests (JabRef#1596) * Cleanup FindFile and rework it using Streams and nio methods- * Unignore test for trying on CI * Use explicit List and Set in findFiles and caller methods * Use Lazy Stream to find files changes should be tested manually Some enhancements and cleanups related to dates (JabRef#1575) * Some enhancements and cleanups related to dates * Fixed some time zone issues * Replaced SimpleDateFormat in ZipFileChooser and replaced arrays with Lists * Changed EasyDateFormat constructors * Fixed stupid mistake * Added CHANGELOG entry * Maybe tests are passing now? * Some server side print debugging... * As it should be * Tryng LocalDateTime * No time zone * Added test for Cookie * Fixed imports... * Added a third possible date format as it turns out that the server changed while developing this PR Builds are now stored via build-upload.jabref.org Consistent file name casing (and other localization improvements) (JabRef#1629) * AUX files * ZIP files * BIB files * JAR files * didn't * Couldn't what's * Consistent casing * AUX apparently is commonly used in French words... * Fixed the flawed quick-and-dirty find-and-replace failures define xjc input/ouput dir (subsequent builds will be faster) (JabRef#1628) Execute task only when input/output dir changed. Fixed a minor issue and refactored MergeEntries (JabRef#1634) * Fixed a minor issue and refactored MergeEntries * Fixed import * Added CHANGELOG entry Added LabelPatternPreferences (JabRef#1607) * Added LabelPatternPreferences * Removed static initializer More tests (JabRef#1635) * Added more tests for Cookie * Enabled some layout tests and added test for StringUtil.intValueOfWithNull * Updated a test * Split tests Updated Errorprone to 2.0.11 (JabRef#1636) * Updated Errorprone to 2.0.11 * Corrected test Keep @comment text in a bib file (JabRef#1638) * Kep @comment text in a bib file * Add test for @comment that contains regular entries Replaced some getField and fixed some bugs (JabRef#1631) * Replaced some getField and fixed some bugs * Fixed a few things * Added CHANGELOG entries * Improved equals implementation * Text book equals and hashCode Fixed JabRef#1639 (JabRef#1641) * Fixed JabRef#1639 * Removed old code Export OO/LO citations to new database (JabRef#1630) * Export OO/LO citations to new database * Fixed problem with duplicates * Added some comments * Fixed spelling in comment * Removed general Exception Unified some equals (JabRef#1640) * Unified some equals * Imported correct Objects... Fixed one more NPE which should have been fixed in JabRef#1631 (JabRef#1649) Finished method to hide visible fields and show hidden fields - Hide method done - Show method done - ToDo repaint hidden field - ToDo test class finished field repaint remove sysouts

Siedlerchr force-pushed the directoryRenamePattern branch 2 times, most recently from 4e8cbe4 to fe2105e Compare July 16, 2016 19:42

oscargus reviewed Jul 17, 2016
View reviewed changes

oscargus reviewed Jul 18, 2016
View reviewed changes

Siedlerchr added cleanup-ops status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Jul 19, 2016

simonharrer reviewed Jul 19, 2016
View reviewed changes

Siedlerchr force-pushed the directoryRenamePattern branch 4 times, most recently from ee8f0a0 to a2e8e28 Compare July 21, 2016 10:07

stefan-kolb reviewed Jul 21, 2016
View reviewed changes

Siedlerchr added 4 commits July 21, 2016 20:46

Cleanup FindFile

a74b16f

Use Lazy Stream to find files Add some more test, reworked test Tests should now work on Linux, too Completey rework File and dir creating using nio methods Fix case typo which was reason for linux bug Move sublass test methods into baseclass hopefully fixes CI build

Use try with resources to close streams

2f6bb08

Set NPE Test to ignore, as it fails only on CI server

Resolve merge conflics

4d6a3ad

Delete Sublass and move methods in base class

750f33b

Hopefully fixes problem for CI Server...

Siedlerchr force-pushed the directoryRenamePattern branch from e633c8f to 750f33b Compare July 21, 2016 18:55

Siedlerchr added 2 commits July 22, 2016 21:06

Merge branch 'master' into directoryRenamePattern

6aef6bd

* master: Added LayoutFormatterPreferences (and related files) (JabRef#1608) [WIP] Create new fetcher infrastructure (JabRef#1594) Set user agent to fix 403 status error More fields added to FieldName (JabRef#1614)

Refactor Stream. Extract BiPredicate

80e812f

Mock Prefs

tobiasdiez reviewed Jul 23, 2016
View reviewed changes

Siedlerchr added 6 commits July 23, 2016 12:58

Remove prefs dependency in findAsscoiatedFiles

2134d98

use TemporaryFolder use Collections.emptyset for expexted

Merge branch 'master' into directoryRenamePattern

4a5eaae

* master: Always use https for help files (JabRef#1615) Resolves JabRef#1613 Use Jabref default icon for uninstaller for now (JabRef#1616) Added more fields and fixed some issues (JabRef#1617)

Checkstyle fix

360cd94

Unignore test for trying on CI

5a7af24

Use explicit List and Set in findFiles and caller methods

0f3d98c

Siedlerchr merged commit 61e9425 into JabRef:master Jul 25, 2016

Siedlerchr deleted the directoryRenamePattern branch July 25, 2016 18:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup FindFile and asssociated tests #1596

Cleanup FindFile and asssociated tests #1596

Siedlerchr commented Jul 16, 2016

Siedlerchr commented Jul 16, 2016

oscargus Jul 17, 2016

oscargus commented Jul 17, 2016

Siedlerchr commented Jul 17, 2016

oscargus Jul 18, 2016

oscargus Jul 18, 2016

Siedlerchr Jul 18, 2016

Siedlerchr Jul 18, 2016

oscargus Jul 18, 2016 via email

Siedlerchr commented Jul 18, 2016

Siedlerchr commented Jul 18, 2016

simonharrer Jul 19, 2016

simonharrer commented Jul 21, 2016

Siedlerchr commented Jul 21, 2016

simonharrer commented Jul 21, 2016

stefan-kolb Jul 21, 2016

tobiasdiez commented Jul 21, 2016

Siedlerchr commented Jul 21, 2016

Siedlerchr commented Jul 21, 2016 •

edited

Loading

oscargus commented Jul 22, 2016

Siedlerchr commented Jul 22, 2016

oscargus commented Jul 22, 2016

simonharrer commented Jul 23, 2016

tobiasdiez Jul 23, 2016

Siedlerchr commented Jul 25, 2016

simonharrer commented Jul 25, 2016

Cleanup FindFile and asssociated tests #1596

Cleanup FindFile and asssociated tests #1596

Conversation

Siedlerchr commented Jul 16, 2016

Siedlerchr commented Jul 16, 2016

Choose a reason for hiding this comment

oscargus commented Jul 17, 2016

Siedlerchr commented Jul 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscargus Jul 18, 2016 via email

Choose a reason for hiding this comment

Siedlerchr commented Jul 18, 2016

Siedlerchr commented Jul 18, 2016

Choose a reason for hiding this comment

simonharrer commented Jul 21, 2016

Siedlerchr commented Jul 21, 2016

simonharrer commented Jul 21, 2016

Choose a reason for hiding this comment

tobiasdiez commented Jul 21, 2016

Siedlerchr commented Jul 21, 2016

Siedlerchr commented Jul 21, 2016 • edited Loading

oscargus commented Jul 22, 2016

Siedlerchr commented Jul 22, 2016

oscargus commented Jul 22, 2016

simonharrer commented Jul 23, 2016

Choose a reason for hiding this comment

Siedlerchr commented Jul 25, 2016

simonharrer commented Jul 25, 2016

Siedlerchr commented Jul 21, 2016 •

edited

Loading