Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add four additional unit tests #7653

Closed
wants to merge 23 commits into from
Closed

Add four additional unit tests #7653

wants to merge 23 commits into from

Conversation

nasdas-dev
Copy link
Contributor

This pull request contributes to issue #6207, which is to add more unit tests to the project.

Tests added:

AppendWordsStrategyTest
ChangeScannerTest
NewEntryActionTest
FileHistoryMenuTest

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

FieldChange.java
18% -> 94%
Abbreviation.java
63% -> 88%
SuggestionProviders.java
0% -> 100%
FileHelper.java
-> Boundary testing of an empty file

CitationKeyGenerator.java
-> Boundary testing of testlagepage parser for 0-00 & 1-1

HTMLCharacterChecker.java
-> Null Value Boundary test
ParsedEntryLink.java
-> Partition testing of ParsingEntryLink

UpperCaseFormatter.java
-> Partition testing for special characters

CitationStyleCacheTest.java
-> Partition testing of cache storage
Checkstyle passed
Assertion Roulette
Added Resource Optimism
Added assertion messages to fix assertion roulette.
General Fixture, removed test code duplication
Fixed AssertionRoulette, one instance of duplicated test code.
Assertion Roulette fixed
fixed Assertion Roulette
using test doubles
@nasdas-dev nasdas-dev changed the title A3 ds Add four additional unit test Apr 20, 2021
@nasdas-dev nasdas-dev changed the title Add four additional unit test Add four additional unit tests Apr 20, 2021
assertEquals("foo", (LayoutEntry.parseMethodsCalls("bla(test),foo(fark)").get(1)).get(0));
assertEquals("test", (LayoutEntry.parseMethodsCalls("bla(test),foo(fark)").get(0)).get(1));
assertEquals("fark", (LayoutEntry.parseMethodsCalls("bla(test),foo(fark)").get(1)).get(1));
assertEquals(1, LayoutEntry.parseMethodsCalls("bla").size(), "Parsing of layout entry method calls failed.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please no comment for the assert, This is bad design. Better group this into multiple methods testing one aspect.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even better, use ParameterizedTests!

@@ -29,41 +39,40 @@ public void migrateToCorrectField(SpecialField field, String fieldInKeyword, Bib

@Test
public void noKewordToMigrate() {
BibEntry entry = new BibEntry().withField(StandardField.AUTHOR, "JabRef")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you change this? withField is a a valid alternative


@Test
void testEquals() {
BibEntry entry = new BibEntry();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We talked about this before in your other PR. No testing on equals or hashcode or toString necessary

assertEquals("bla", (LayoutEntry.parseMethodsCalls("bla").get(0)).get(0), "Parsing of layout entry method calls failed. Input did not match expected output.");

assertEquals(1, LayoutEntry.parseMethodsCalls("bla,").size(), "Parsing of layout entry method calls failed. Check for comma behaviour.");
assertEquals("bla", (LayoutEntry.parseMethodsCalls("bla,").get(0)).get(0), "Parsing of layout entry method calls failed. Input did not match expected output.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Directly compare the content of the lists, not their size!

@Siedlerchr Siedlerchr added the status: changes required Pull requests that are not yet complete label Apr 20, 2021
@Siedlerchr
Copy link
Member

It seems like we already have enough unit tests for that classes and here are many confilicts, so Im closing

@Siedlerchr Siedlerchr closed this May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: changes required Pull requests that are not yet complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants