Real test linking real other entry #3214
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Siedlerchr
reviewed
Sep 13, 2017
| String expectedCrossrefValue = targetCiteKey.get(); | ||
| crossrefs = actualcrossrefValue.contains(expectedCrossrefValue); | ||
| } | ||
| assertTrue(crossrefs); |
There was a problem hiding this comment.
Are you aware that you can directly call assertEquals on two optionals?
https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html#equals-java.lang.Object-
Siedlerchr
reviewed
Sep 13, 2017
|
|
||
| @Test | ||
| public void givenTargetAndSourceWhenSourceCrossrefTargetThenSourceCrossrefsTarget() { | ||
| target = BibEntryBuild.ing().withId("target").withCiteKey("target").now(); |
There was a problem hiding this comment.
I don't see why it's necessary to have a BibEntry build class?
Why not just simply create two new BibEntrys? And you won't need the actual database file.
e.g.
database = new BibDatabase()
BibEntry source = new BibEntry()
source.setCiteKey("source")
source.setField(Crossref, "target")
database.insert(source)
BibEntry target = new BibEntry()
target.setCiteKey("target")
database.insert(target)
There was a problem hiding this comment.
Put that in the before method and it will be "freshly created" for every test
There was a problem hiding this comment.
BibEntry actually contains a withField method if you want to use a more fluent api,
jabref/src/main/java/org/jabref/model/entry/BibEntry.java
Lines 734 to 737 in 23a0610
If you want, you can also add a new method
withCiteKey along these lines.
Siedlerchr
requested changes
Sep 13, 2017
added 5 commits
September 14, 2017 09:21
|
Thank you for your valuable feedback, I learned a lot and implemented all change requests |
LinusDietz
approved these changes
Sep 14, 2017
Siedlerchr
added a commit
that referenced
this pull request
Sep 15, 2017
* upstream/master: Add tests for removal changes Improve telemetry (#3218) Real test linking real other entry (#3214) Check for explicit group at different location Update java-string-similarity from 0.24 -> 1.0.0 Only remove ExplicitGroups from entries, but not keyword-based groups Remove unused import in GroupTreeNode Move getEntriesInGroup to GroupTreeNode Inline Consumer updateViewModel Undo IntelliJ autoformat of FXML annotations Remove unused import Rewrite selection logic to avoid NPEs Clear group selection when a group is removed Also remove a group from entries when you remove it
sammann-fnordkollektiv
deleted the
real-test-linking-real-other-entry
branch
Siedlerchr
added a commit
that referenced
this pull request
Sep 23, 2017
* upstream/master: (188 commits) Show file description only if not empty Add file description to gui and fix sync bug (#3210) Don't highlight odd rows in file list editor (#3223) Fix assertion by removing typo (#3220) Update assertion to change of online reference (#3221) Put in null return Reformatted code, renamed method, added try/catch Add tests for removal changes Improve telemetry (#3218) Real test linking real other entry (#3214) Check for explicit group at different location Update java-string-similarity from 0.24 -> 1.0.0 Only remove ExplicitGroups from entries, but not keyword-based groups Localization: French: Translation of a new string (#3212) Fix null return Changed from Path to Optional<Path> Fix #2775: Hyphens in last names are properly parsed (#3209) Followup to Issue #3167 (#3202) Remove unused import in GroupTreeNode Move getEntriesInGroup to GroupTreeNode ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
gradle localizationUpdate?Why
Based on #3191 (comment) I added two tests concerning cross referencing.
What
Watchout
assertSourceCrossrefsTargetfurther due to theOptional<>concept.BibEntryBuild.It is an inner class, because I am uncertain where to put it.DatabaseFromFileto keep test code readable. It is an inner class, because I did not find a similar wrapper nor did I know where to put it. Its construction uses a path to a file to be parsed to a database.