test: enhanced RemoveLinksToNotExistentFilesTest #3

burcukilic · 2024-03-03T16:17:44Z

No description provided.

src/test/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFilesTest.java

karlsb · 2024-03-03T16:44:09Z

src/test/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFilesTest.java

        List<FieldChange> changes = removeLinks.cleanup(entry);
+
        assertFalse(changes.isEmpty());


I suspect that we may get feedback on these kinds of asserts. Rather than just checking that there are changes being returned. The ideal would be to expect some change:

List<FieldChange> expectedChanges = ... assertEquals(expectedChanges, changes);

They should be comparable through the equals method in FieldChange.java otherwise maybe we could use the toString() method.

I think the assertion with expectedEntry already does this

I believe that the wanted us to change this assertion specifically and I don't think expectedEntry currently covers this completely.

src/test/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFilesTest.java

LinusWallin · 2024-03-03T18:02:08Z

src/main/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFiles.java

        boolean changed = false;
        for (LinkedFile file : files) {
            LinkedFileHandler fileHandler = new LinkedFileHandler(file, entry, databaseContext, filePreferences);

-            if (file.isOnlineLink() == false) {
+            if (!file.isOnlineLink()) {
+


the empty line after the if statement will most likely result in formatting error

LinusWallin · 2024-03-03T18:05:19Z

src/test/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFilesTest.java

        List<FieldChange> changes = removeLinks.cleanup(entry);
+


a lot of empty lines, will cause errors

LinusWallin · 2024-03-03T18:05:32Z

src/test/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFilesTest.java

+
+

empty lines

LinusWallin · 2024-03-03T18:06:03Z

src/test/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFilesTest.java

+
+        assertEquals(2, entry.getFiles().size()); //Since no files deleted the linked files should remain 2
+
+        assertTrue(entry.getFiles().get(0).isOnlineLink());
+        assertFalse(entry.getFiles().get(1).isOnlineLink());
+
+        assertEquals(expectedEntry, entry); 
    }
+
+


more empty lines

LinusWallin · 2024-03-03T18:06:23Z

src/test/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFilesTest.java

+
+        assertEquals(1, entry.getFiles().size());
+        assertTrue(entry.getFiles().get(0).isOnlineLink()); //The non-deleted file should be linked online
+
+        assertEquals(expectedEntry, entry); 
    }

    @Test
    void keepLinksToExistingFiles() {
+
+        LinkedFile fileField = new LinkedFile("", fileBefore.toAbsolutePath(), "");
+        BibEntry expectedEntry = new BibEntry(StandardEntryType.Article)
+                .withField(StandardField.AUTHOR, "Shatakshi Sharma and Bhim Singh and Sukumar Mishra")
+                .withField(StandardField.DATE, "April 2020")
+                .withField(StandardField.YEAR, "2020")
+                .withField(StandardField.DOI, "10.1109/TII.2019.2935531")
+                .withField(StandardField.FILE, FileFieldWriter.getStringRepresentation(Arrays.asList(
+                    new LinkedFile("", "https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=8801912:PDF", ""),
+                    fileField)))
+                .withField(StandardField.ISSUE, "4")
+                .withField(StandardField.ISSN, "1941-0050")
+                .withField(StandardField.JOURNALTITLE, "IEEE Transactions on Industrial Informatics")
+                .withField(StandardField.PAGES, "2346--2356")
+                .withField(StandardField.PUBLISHER, "IEEE")
+                .withField(StandardField.TITLE, "Economic Operation and Quality Control in PV-BES-DG-Based Autonomous System")
+                .withField(StandardField.VOLUME, "16")
+                .withField(StandardField.KEYWORDS, "Batteries, Generators, Economics, Power quality, State of charge, Harmonic analysis, Control systems, Battery, diesel generator (DG), distributed generation, power quality, photovoltaic (PV), voltage source converter (VSC)");
+
+        assertEquals(2, entry.getFiles().size());
+


more empty lines that have to be removed

LinusWallin · 2024-03-03T18:12:23Z

src/test/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFilesTest.java

        List<FieldChange> changes = removeLinks.cleanup(entry);
+
        assertFalse(changes.isEmpty());


I believe that the wanted us to change this assertion specifically and I don't think expectedEntry currently covers this completely.

formatted files according to checkstyle plugin.

* Adds feature CleanupJob for removing non-existent files (#2) * feat: RemoveLinksToNotExistentFiles.java * feat: added option to cleanup entries * feat: improved cleanup entry dialogue * Test: implements test for linked files which exist Implementation of a test which calls the cleanup functionality of RemoveLinksToNotExistentFiles with a entry that is linked to an existing file. The test makes sure that no changes are made since the linked file exists. * Refactor: refactored test to match the standard of the project Removed the JavaDoc comment from the test, since other tests in the project don't have JavaDoc:s. Also removed test from the name of the test function, as it seems that the standard way of naming tests is to not have test in the testfunctions name. * Refactor: removed system.out.print() Removed an print from the tests. * fix: non english character deleted * Docs: Added information about changes Added information about the changes made for solving issue#10929 to the CHANGES.md file. --------- Co-authored-by: burcukilic <94201593+burcukilic@users.noreply.github.com> * Fix: Removes unused imports and solves other errors Removes unused imports and solves the error of language keys that were not in the english laguage file. * Fix: Removes empty line and solves other issues Removes an empty line where 2 empty lines appear in after each other. Also adds brackets to URL and removes an key which is not in the english language file and was missed in the last commit. * Docs: Removes technicality from changelog Removes technical details for the change made in changelog and also follows the suggestion for removing the space and brackets between the link and the issue. * test: enhanced RemoveLinksToNotExistentFilesTest (#3) * test: enhanced RemoveLinksToNotExistentFilesTest * test: add real content * test: change structure * test: added expectedChanges * test: added a third test to RemoveLinksToNotExistentFilesTest * refactor: removed unnecessary toString * style: removed a whitespace * refactor: checkstyle formatting. formatted files according to checkstyle plugin. --------- Co-authored-by: karlsb <36365664+karlsb@users.noreply.github.com> * Resolve request changes (#4) * test: enhanced RemoveLinksToNotExistentFilesTest * test: add real content * test: change structure * test: added expectedChanges * test: added a third test to RemoveLinksToNotExistentFilesTest * refactor: removed unnecessary toString * style: removed a whitespace * refactor: checkstyle formatting. formatted files according to checkstyle plugin. * refactor: replaced .get(0) with .getFirst() for first accessing element of List. Changed was required from ./gradlew rewriteDryRun --------- Co-authored-by: burcukilic <94201593+burcukilic@users.noreply.github.com> * Resolve more request changes and warnings (#5) * Reverse the condition and the content in RemoveLinksToNotExistentFiles Resolves comment "Reverse the condition and the content. Reason: You have BOTH (true and false) cases covered. The true case should normally come first." * Fix: Store bibFolder.resolve("test.bib") in a variable * Refactor: Rename variable fileFolder to originalFileFolder and remove comment. * Refactor: Rename defaultFileFolder to newFileFolder * Refactor: Fix indentation in RemoveLinksToNotExistentFilesTest * Refactor: Replaced Arrays.asList() with List.of() * Refactor: Replaced Arrays.asList() with List.of() * Refactor: Change to use java.nio for file deletion. * Refactor: Move comment to line above. * Refactor: Added "PDF" as third argument to LinkedFile for OnlineLink files * Refactor: Removed unused variables in RemoveLinksToNotExistentFiles * Refactor: Throw IOException in RemoveLinksToNotExistentFilesTest functions * Refactor: Tests assert using List.of() in RemoveLinksToNotExistentFilesTest * Refactor: Corrected Arguments for LinkedFile in RemoveLinksToNotExistentFilesTest * Refactor: Class fields into local variables This refactor fixed 3 warnings. * docs: updated docs to remove merge conflict --------- Co-authored-by: Linus Wallin <73783914+LinusWallin@users.noreply.github.com> Co-authored-by: burcukilic <94201593+burcukilic@users.noreply.github.com> Co-authored-by: Linus Wallin <linuswallin@live.se>

test: enhanced RemoveLinksToNotExistentFilesTest

b292637

karlsb reviewed Mar 3, 2024

View reviewed changes

src/test/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFilesTest.java Outdated Show resolved Hide resolved

karlsb reviewed Mar 3, 2024

View reviewed changes

src/test/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFilesTest.java Show resolved Hide resolved

karlsb reviewed Mar 3, 2024

View reviewed changes

src/test/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFilesTest.java Outdated Show resolved Hide resolved

burcukilic added 2 commits March 3, 2024 17:40

test: add real content

02288ce

test: change structure

8899ea3

LinusWallin reviewed Mar 3, 2024

View reviewed changes

test: added expectedChanges

f3a71ec

karlsb approved these changes Mar 3, 2024

View reviewed changes

burcukilic and others added 4 commits March 3, 2024 19:26

test: added a third test to RemoveLinksToNotExistentFilesTest

88b8704

refactor: removed unnecessary toString

120cab4

style: removed a whitespace

5e17189

refactor: checkstyle formatting.

ee306c9

formatted files according to checkstyle plugin.

karlsb merged commit 2ba3abd into main Mar 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: enhanced RemoveLinksToNotExistentFilesTest #3

test: enhanced RemoveLinksToNotExistentFilesTest #3

burcukilic commented Mar 3, 2024

karlsb Mar 3, 2024

burcukilic Mar 3, 2024

LinusWallin Mar 3, 2024

LinusWallin Mar 3, 2024

LinusWallin Mar 3, 2024

LinusWallin Mar 3, 2024

LinusWallin Mar 3, 2024

LinusWallin Mar 3, 2024

LinusWallin Mar 3, 2024

		List<FieldChange> changes = removeLinks.cleanup(entry);

		assertFalse(changes.isEmpty());

test: enhanced RemoveLinksToNotExistentFilesTest #3

test: enhanced RemoveLinksToNotExistentFilesTest #3

Conversation

burcukilic commented Mar 3, 2024

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

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

Choose a reason for hiding this comment