Added tests for DatabaseFileLookup #11108

arfazhxss · 2024-03-29T07:50:53Z

I have added new test cases directoryPathTests to the DatabaseFileLookupTest class. These test cases verifies the functionality of the DatabaseFileLookup class in handling file paths and directories. It creates a temporary directory and file, sets up a BibDatabaseContext with two entries (one with a valid file path and one with an empty file path), and configures the FilePreferences to use the temporary directory as the default file directory. Then, it creates an instance of DatabaseFileLookup and asserts that the file in the temporary directory is found, while a non-existent file is not found. The test case checks if the FilePreferences are correctly set up with the temporary directory as the main file directory, and if the getPathOfDatabase() method of DatabaseFileLookup returns an empty path.

Mandatory checks

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 developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Added tests for DatabaseFileLookup

koppor · 2024-03-29T08:56:24Z

Please first fix tests to enable easier reviewing. You see the failing tests at the red crosses of the page of this PR. @arfazhxss

koppor · 2024-03-29T09:42:38Z

I think, AuthirListTest should not be part of the PR

koppor

This test is a good start. It misses testing of different configurated file directies. Please re-iterate in the documentation.

In numbers: 20% of the issue solved.

src/test/java/org/jabref/logic/importer/DatabaseFileLookupTest.java

koppor · 2024-03-29T09:54:34Z

@arfazhxss in future, do NOT use the main branch for cide changes. Create a separate branch for each issue solved!

arfazhxss · 2024-03-29T10:05:35Z

@arfazhxss in future, do NOT use the main branch for cide changes. Create a separate branch for each issue solved!

Thank you for the advice:) I am working on fixing the errors from the tests.

Siedlerchr · 2024-03-29T15:28:25Z

Please read the devdocs and set up your code style https://devdocs.jabref.org/getting-into-the-code/guidelines-for-setting-up-a-local-workspace/intellij-13-code-style.html

arfazhxss · 2024-03-29T16:05:02Z

@Siedlerchr I have one failing checks for the checkStyle for now:

DatabaseFileLookupTest.java:10:1: Wrong order for 'java.nio.file.Files' import. [ImportOrder]
DatabaseFileLookupTest.java:18:1: 'javafx.collections.FXCollections' should be separated from previous imports. [ImportOrder]
DatabaseFileLookupTest.java:20:1: 'org.jabref.logic.importer.fileformat.BibtexImporter' should be separated from previous imports. [ImportOrder]
DatabaseFileLookupTest.java:28:1: 'org.junit.jupiter.api.BeforeEach' should be separated from previous imports. [ImportOrder]

I think I might need a bit help on this one.

arfazhxss · 2024-03-29T16:10:36Z

For now, the tests are covering 100% of classes, lines and methods. For the branch, the tests haven't been added for the .filter() method in the parseFileField() function.

Siedlerchr · 2024-03-29T16:35:20Z

In Intellij menu -> Code Reformat code and optimize imports

src/test/java/org/jabref/logic/importer/DatabaseFileLookupTest.java

koppor

Thank you for working on this

It does not fulfill the aims of the issue. JabRef has four places to store PDFs: Next to the bib file, in the general file directory, database file directory, the user-specific file directory. I don't see tests for all these cases.

CHANGELOG.md

src/test/java/org/jabref/logic/importer/DatabaseFileLookupTest.java

arfazhxss · 2024-04-01T20:26:06Z

I will keep working on this pull request throughout next couple of weeks

arfazhxss · 2024-04-09T06:22:34Z

In my latest changes, I fixed the following requested changes:

Removed redundant comments
Added @tempdir from JUnit
Modified List.of from Array Implementation
Using the actual BibDatabase, BibDatabaseContext and Metadata Implementation
One assertion per test
CheckStyle corrections, optimized imports and code reformating
Added test cases for general file directory, database file directory, the user-specific file directory and bib database file directory

@koppor Please let me know how I can improve the test cases if needed.

koppor

I am so sorry that my "hints" section at JabRef#678 made the impression that the task list was complete.

The intention of the task was that students think of test cases.

Here some hints:

JabRef has 3 different file directories
- global one
- library-specific
- user-specific

Thus, file can be placed in

global one
library-specific
user-specific
directory of other user

File can exist in multiple directories.

The work should cover all cases derived from that.

E.g.,

file in user-specific directory and global directory. Found in user-specific dir.
file in user-specific directory and library-specific directory. Found in user-specific dir.
file in user-specific directory and library-specific directory and global directory. Found in user-specific dir.
file in user-specific directory of other user and library-specific directory and global directory. Found in library-specific dir.
file in user-specific directory of other user and global directory. Found in global dir.
file in library-specific directory and global directory. Found in library-specific dir.
File in global-directory only. Found there.
File in user-specific directory. Found there.
File in library-specific directory. Found there.

This what I meant that 20% of the work have been done.

src/test/java/org/jabref/logic/importer/DatabaseFileLookupTest.java

arfazhxss · 2024-04-11T07:33:15Z

JabRef has 3 different file directories

global one

library-specific

user-specific

Thus, file can be placed in

global one

library-specific

user-specific

directory of other user

File can exist in multiple directories.

The work should cover all cases derived from that.

@koppor thanks for clarifying! I want to get some more clarification on my test cases (and how I can improve them). My current test cases makes an x.txt and passes it through the metadata and databaseContext, and then uses DatabaseFileLookup to test whether the file is correctly detected in the database. It also checks if that file exists in the general file directory, database file directory, the user-specific file directory and bib/library file directory. Does it mean that my current test cases covers the following marked-cases only for now?

file in user-specific directory and global directory. Found in user-specific dir.
file in user-specific directory and library-specific directory. Found in user-specific dir.
file in user-specific directory and library-specific directory and global directory. Found in user-specific dir.
file in user-specific directory of other user and library-specific directory and global directory. Found in library-specific dir.
file in user-specific directory of other user and global directory. Found in global dir.
file in library-specific directory and global directory. Found in library-specific dir.
File in global-directory only. Found there.
File in user-specific directory. Found there.
File in library-specific directory. Found there.

This is just to clarify if I am on the right track.

koppor

Your whole tests need to be reorganized. Sorry for this.

One test for case.

Example test names and content:

librarySpecificDirectoryOnly. Places the file in library-specific directory. Checks that this file is found.
userSpecificDirectoryAndLibrarySpecificDirectory. Pleases the file in both user-specific and library-specific directory. Asserts that the file is found in the user-specific directory.

Note that the file is stored as filename without path.

Thus txtFileDirPath = txtFileDir.toAbsolutePath() is absolutely wrong!!

The part of the BibEntry is file = {:test.pdf}. And JabRef searches for that file.

(If you use absolute paths, it is boring, because JabRef can also ignore the directories, because the path is absolute. It is about resolving)

koppor · 2024-04-11T08:34:16Z

src/test/java/org/jabref/logic/importer/DatabaseFileLookupTest.java

        assertEquals(2, entries.size());
        assertNotNull(entry1);
        assertNotNull(entry2);


A test case should focus on one thing. Not testing that constructors of data classes work. Remove all three assertions.

koppor · 2024-04-11T08:34:23Z

src/test/java/org/jabref/logic/importer/DatabaseFileLookupTest.java

+     */
+    @Test
+    void fileShouldBeFound() {
+        assertTrue(fileLookup.lookupDatabase(txtFileDir));


This assertion is OK.

koppor · 2024-04-11T08:34:37Z

src/test/java/org/jabref/logic/importer/DatabaseFileLookupTest.java

+    @Test
+    void fileShouldBeFound() {
+        assertTrue(fileLookup.lookupDatabase(txtFileDir));
+        assertNotNull(fileLookup.getPathOfDatabase());


Remove this. Tests other aspects.

koppor · 2024-04-11T08:35:05Z

src/test/java/org/jabref/logic/importer/DatabaseFileLookupTest.java

+    /**
+     * x.txt should be found in the user-specific directory
+     */
+    @Test
+    void userSpecificDirectory() {
+        assertEquals(metaData.getUserFileDirectory(filePreferences.getUserAndHost()), Optional.of(txtFileDirPath));
+    }


description does not match assertion (does it)?

koppor · 2024-04-11T08:46:24Z

@arfazhxss I refined the issue to be more clear. Look at "User-facing thing" at JabRef#678. Hope, that helps to understand the issue.

koppor · 2024-04-11T08:47:46Z

My current test cases makes an x.txt

As far as I understand, you put an absolute path. Then, nothing of JabRef's lookup functionality is triggered. Story x.txt as is in StandardFields.File and see what happens.

koppor · 2024-04-29T10:04:45Z

@arfazhxss I did not see any activity the last weeks. We need to keep our number of opened pull requests low to enable maintaining the project. If you continue on this, feel free to open a new pull request.

arfazhxss added 4 commits March 29, 2024 00:27

Update DatabaseFileLookupTest.java

74a161d

Update CHANGELOG.md

b0f3368

Merge pull request #1 from arfazhxss/arfx/DatabaseFileLookupTests

f281d6c

Added tests for DatabaseFileLookup

Merge branch 'main' into main

4f9a9ea

koppor requested changes Mar 29, 2024

View reviewed changes

arfazhxss force-pushed the main branch from 7552a34 to 4f9a9ea Compare March 29, 2024 09:51

arfazhxss added 2 commits March 29, 2024 08:06

Update DatabaseFileLookupTest.java

cf41e4d

Update DatabaseFileLookupTest.java

4e710c8

Minor CheckStyle Corrections

b810d99

Siedlerchr reviewed Mar 29, 2024

View reviewed changes

src/test/java/org/jabref/logic/importer/DatabaseFileLookupTest.java Outdated Show resolved Hide resolved

arfazhxss force-pushed the main branch from aa46ba7 to b810d99 Compare March 29, 2024 23:00

Optimized Imports

1beaddd

arfazhxss requested review from koppor and Siedlerchr March 30, 2024 00:21

koppor requested changes Mar 30, 2024

View reviewed changes

Update CHANGELOG.md

145376e

koppor reviewed Mar 30, 2024

View reviewed changes

src/test/java/org/jabref/logic/importer/DatabaseFileLookupTest.java Outdated Show resolved Hide resolved

Merge branch 'main' into main

46627d3

koppor marked this pull request as draft April 1, 2024 21:32

arfazhxss added 2 commits April 8, 2024 18:21

Merge branch 'main' into main

0e9a310

Update DatabaseFileLookupTest.java

2507209

arfazhxss added 2 commits April 8, 2024 20:34

Removing Unused Imports

a92ec91

Added main, general, user-specific and bib directory tests

2e8404f

arfazhxss requested a review from koppor April 9, 2024 06:22

arfazhxss marked this pull request as ready for review April 9, 2024 06:23

arfazhxss added 2 commits April 9, 2024 09:40

Merge branch 'main' into main

f79239b

Merge branch 'main' into main

04e220c

koppor requested changes Apr 11, 2024

View reviewed changes

src/test/java/org/jabref/logic/importer/DatabaseFileLookupTest.java Outdated Show resolved Hide resolved

Remove redundant createTempDirectory() method

cfe1061

arfazhxss marked this pull request as draft April 11, 2024 07:49

koppor requested changes Apr 11, 2024

View reviewed changes

Merge branch 'main' into main

db3d60a

koppor closed this Apr 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added tests for DatabaseFileLookup #11108

Added tests for DatabaseFileLookup #11108

arfazhxss commented Mar 29, 2024 •

edited

Loading

koppor commented Mar 29, 2024

koppor commented Mar 29, 2024

koppor left a comment

koppor commented Mar 29, 2024

arfazhxss commented Mar 29, 2024

Siedlerchr commented Mar 29, 2024

arfazhxss commented Mar 29, 2024 •

edited

Loading

arfazhxss commented Mar 29, 2024

Siedlerchr commented Mar 29, 2024

koppor left a comment

arfazhxss commented Apr 1, 2024

arfazhxss commented Apr 9, 2024 •

edited

Loading

koppor left a comment

arfazhxss commented Apr 11, 2024

koppor left a comment

koppor Apr 11, 2024

koppor Apr 11, 2024

koppor Apr 11, 2024

koppor Apr 11, 2024

koppor commented Apr 11, 2024

koppor commented Apr 11, 2024

koppor commented Apr 29, 2024

Added tests for DatabaseFileLookup #11108

Added tests for DatabaseFileLookup #11108

Conversation

arfazhxss commented Mar 29, 2024 • edited Loading

Mandatory checks

koppor commented Mar 29, 2024

koppor commented Mar 29, 2024

koppor left a comment

Choose a reason for hiding this comment

koppor commented Mar 29, 2024

arfazhxss commented Mar 29, 2024

Siedlerchr commented Mar 29, 2024

arfazhxss commented Mar 29, 2024 • edited Loading

arfazhxss commented Mar 29, 2024

Siedlerchr commented Mar 29, 2024

koppor left a comment

Choose a reason for hiding this comment

arfazhxss commented Apr 1, 2024

arfazhxss commented Apr 9, 2024 • edited Loading

koppor left a comment

Choose a reason for hiding this comment

arfazhxss commented Apr 11, 2024

koppor left a comment

Choose a reason for hiding this comment

koppor Apr 11, 2024

Choose a reason for hiding this comment

koppor Apr 11, 2024

Choose a reason for hiding this comment

koppor Apr 11, 2024

Choose a reason for hiding this comment

koppor Apr 11, 2024

Choose a reason for hiding this comment

koppor commented Apr 11, 2024

koppor commented Apr 11, 2024

koppor commented Apr 29, 2024

arfazhxss commented Mar 29, 2024 •

edited

Loading

arfazhxss commented Mar 29, 2024 •

edited

Loading

arfazhxss commented Apr 9, 2024 •

edited

Loading