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.

Sign up for GitHub

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

Jump to bottom

67 test rest interface #76

Open

vinnyt123 wants to merge 11 commits into jabmap from 67-test-REST-interface

Collaborator

vinnyt123 commented Sep 29, 2020 •

edited

Loading

Tests created for GET /libraries/current/entries
Tests created for PUT /libraries/current/map
Tests created for GET /libraries/current/map

vinnyt123 and others added 11 commits

September 27, 2020 14:14


          Add test setup for RootResource

98565a3


          Finish tests for save mind map

cc977df


          GET test: Get MindMap consisting of single node

dcd0b96


          GET test: Get MindMap consisting of multiple nodes and edges

36e907e


          Refactor DatabaseAccess and add JabMapHTTPServer


          Finish getEntries tests

3d3a279


          GET test: No mindmaps in databae

0c76e30


          Merge remote-tracking branch 'origin/67-test-REST-interface' into 67-…

84187c4

…test-REST-interface


          GET tests: Only reference bib entries and both reference and mindmap …

45a3cb0

…bib entries


          Refactoring

4133ea0


          Clean up test code and refactor rest classes into better package stru…

e0d4988

…cture

vinnyt123 changed the title ~~[WIP] 67 test rest interface~~ 67 test rest interface

Collaborator

koppor commented Oct 8, 2020

This fixes #67

koppor requested changes

View reviewed changes

Collaborator

koppor left a comment

In general LGTM - some nitpicks

src/main/java/org/jabref/gui/JabRefMain.java

@@ @@ -25,12 +25,9 @@ @@
               import org.jabref.migrations.PreferencesMigrations;
               import org.jabref.model.database.BibDatabaseMode;
               import org.jabref.preferences.JabRefPreferences;
-              import org.jabref.rest.resources.RootResource;
+              import org.jabref.rest.jabmap.JabMapHTTPServer;

Collaborator

koppor Oct 8, 2020

Please name the calss JabMapHttpServer. JabRef tries to follow https://google.github.io/styleguide/javaguide.html#s5.3-camel-case

src/test/java/org/jabref/logic/jabmap/BibtexMindMapAdapterTest.java

-                      // assertEquals(List.of(node1Entry, node2Entry, edgeEntry), entries);
-                  }
-              }
+              //package org.jabref.logic.jabmap;

Collaborator

koppor Oct 8, 2020

Why is the test disabled?

src/test/java/org/jabref/rest/jabmap/resources/RootResourceTest.java

+                  @BeforeEach
+                  void initMockDatabase() {
+                      RootResource.databaseAccess = new MockDatabaseAccess();

Collaborator

koppor Oct 8, 2020

Can we work with injection somehow here? @calixtus do you have an idea how to do that?

src/test/java/org/jabref/rest/jabmap/resources/RootResourceTest.java

+              import java.util.Arrays;
+              import java.util.List;
+              import javax.ws.rs.client.Client;

Collaborator

koppor Oct 8, 2020

The test does not use REST-assured as proposed at #67 (comment). An ADR should be added for that.

src/test/java/org/jabref/rest/jabmap/resources/RootResourceTest.java

+                          response = client.target(WEB_SERVICE_URI).path("libraries/current/entries").request().accept(MediaType.APPLICATION_JSON).get();
+                          assertEquals(200, response.getStatus());
+                          String responseBody = response.readEntity(String.class);
+                          String expectedJson = "[{\"type\":{\"bibtex_metadata\":\"Article\",\"key\":\"article1\"}},{\"type\":{\"bibtex_metadata\":\"Book\",\"key\":\"book1\"}},{\"type\":{\"bibtex_metadata\":\"MastersThesis\",\"key\":\"MastersThesis1\"}}]";

Collaborator

koppor Oct 8, 2020 •

edited

Loading

This will be a maintenance hell, because the whitespace and key ordering does not matter for equality.

I think, JSONassert should be used. --> https://github.com/skyscreamer/JSONassert. Then only one line needs to be changed --> assertEquals --> JSONAssert.assertEquals.

Maybe try with Jackson or another JSON library for a equality? --> https://www.baeldung.com/jackson-compare-two-json-objects

It does not seem possible with REST-assured to check for the complete json directly.

src/test/java/org/jabref/rest/jabmap/resources/RootResourceTest.java

+                          assertEquals(expectedJson, responseBody);
+                      } finally {
+                          response.close();

Collaborator

koppor Oct 8, 2020

This reads like the try-with-resoures (https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html) should be used

src/test/java/org/jabref/rest/jabmap/resources/RootResourceTest.java

+                      Response response = null;
+                      try {
+                          response = client.target(WEB_SERVICE_URI).path("libraries/current/entries").request().accept(MediaType.APPLICATION_JSON).get();
+                          assertEquals(200, response.getStatus());

Collaborator

koppor Oct 8, 2020

This is a call for REST-assured. Maybe, the code can be quickly rewritten to use REST-assured?

String responseString = get(uri + "/movie/" + testMovie.getId()).then()
	  .assertThat()
	  .statusCode(HttpStatus.OK.value())
	  .extract()
	  .asString();

(Source: https://www.baeldung.com/rest-assured-response)

koppor reviewed

View reviewed changes

src/test/java/org/jabref/rest/jabmap/resources/RootResourceTest.java

+                          response = client.target(WEB_SERVICE_URI).path("libraries/current/entries").request().accept(MediaType.APPLICATION_JSON).get();
+                          assertEquals(200, response.getStatus());
+                          String responseBody = response.readEntity(String.class);
+                          String expectedJson = "[{\"type\":{\"bibtex_metadata\":\"Article\",\"key\":\"article1\"}},{\"type\":{\"bibtex_metadata\":\"Book\",\"key\":\"book1\"}},{\"type\":{\"bibtex_metadata\":\"MastersThesis\",\"key\":\"MastersThesis1\"}}]";

Collaborator

koppor Oct 9, 2020

Please change the JSON format to a simpley key/value map as outlined at #73 (comment).

Collaborator

koppor commented Oct 9, 2020

Please merge upstream/master to get most tests fixed. Please also read on at the failing tests, you'll find

org.jabref.logic.l10n.LocalizationConsistencyTest

  Test findMissingLocalizationKeys() FAILED (1.3s)

  org.opentest4j.AssertionFailedError: DETECTED LANGUAGE KEYS WHICH ARE NOT IN THE ENGLISH LANGUAGE FILE
  PASTE THESE INTO THE ENGLISH LANGUAGE FILE
  
  Launch JabMap=Launch JabMap
  Open JabMap=Open JabMap
   ==> expected: <[]> but was: <[Launch JabMap (src/main/java/org/jabref/gui/keyboard/KeyBinding.java LANG), Open JabMap (src/main/java/org/jabref/gui/keyboard/KeyBinding.java LANG)]>
      at org.jabref.logic.l10n.LocalizationConsistencyTest.findMissingLocalizationKeys(LocalizationConsistencyTest.java:114)

By "ENGLISH LANGUAGE FILE", JabRef_en.properties is meant.

koppor mentioned this pull request

[WIP] Rhar768/feature/23 rest interface #59

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet