From 1259d3e9bcc3c2c2630295eff3517db2a1f7b8e1 Mon Sep 17 00:00:00 2001 From: Joshua Send Date: Fri, 7 Aug 2020 15:00:40 +0100 Subject: [PATCH] Migrate Client Tests From Core (#149) ## What is the goal of this PR? We migrate one test and implement BDD steps required for https://github.com/graknlabs/verification/pull/88 which migrated client-java tests from core to BDD as well. ## What are the changes implemented in this PR? * Implement new `TransactionSteps` methods * implement new `KeyspaceSteps` method * Migrate a concurrecny test to `AnswerIT` -- should be in ACID tests in the future --- dependencies/graknlabs/repositories.bzl | 2 +- test/assembly/QueryTest.java | 1 - test/behaviour/connection/keyspace/BUILD | 5 ++- .../connection/keyspace/KeyspaceSteps.java | 15 ++++++- test/behaviour/connection/transaction/BUILD | 2 + .../transaction/TransactionSteps.java | 42 ++++++++++++++++++- test/behaviour/graql/GraqlSteps.java | 9 +++- test/integration/answer/AnswerIT.java | 37 ++++++++-------- 8 files changed, 89 insertions(+), 24 deletions(-) diff --git a/dependencies/graknlabs/repositories.bzl b/dependencies/graknlabs/repositories.bzl index cd86357a06..e86e41f21a 100644 --- a/dependencies/graknlabs/repositories.bzl +++ b/dependencies/graknlabs/repositories.bzl @@ -51,7 +51,7 @@ def graknlabs_verification(): git_repository( name = "graknlabs_verification", remote = "https://github.com/graknlabs/verification", - commit = "e1cce9175641d864be0306c21fc6c495b87c24e4", # sync-marker: do not remove this comment, this is used for sync-dependencies by @graknlabs_verification + commit = "2ec55271b4c324018897754e7d9294e0d8ea911a", # sync-marker: do not remove this comment, this is used for sync-dependencies by @graknlabs_verification ) def graknlabs_grabl_tracing(): diff --git a/test/assembly/QueryTest.java b/test/assembly/QueryTest.java index a59850ea84..07a0cc07eb 100644 --- a/test/assembly/QueryTest.java +++ b/test/assembly/QueryTest.java @@ -35,7 +35,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.File; import java.io.IOException; import java.util.List; import java.util.concurrent.TimeoutException; diff --git a/test/behaviour/connection/keyspace/BUILD b/test/behaviour/connection/keyspace/BUILD index 6b4906d9cc..e0733e93b2 100644 --- a/test/behaviour/connection/keyspace/BUILD +++ b/test/behaviour/connection/keyspace/BUILD @@ -50,7 +50,10 @@ grakn_test( "@maven//:io_cucumber_cucumber_junit", ], runtime_deps = [ - ":steps" + ":steps", + "//test/behaviour/connection/session:steps", + "//test/behaviour/connection/transaction:steps", + "//test/behaviour/config:parameters", ], data = [ "@graknlabs_verification//behaviour/connection:keyspace.feature", diff --git a/test/behaviour/connection/keyspace/KeyspaceSteps.java b/test/behaviour/connection/keyspace/KeyspaceSteps.java index 81896aef63..beda220764 100644 --- a/test/behaviour/connection/keyspace/KeyspaceSteps.java +++ b/test/behaviour/connection/keyspace/KeyspaceSteps.java @@ -34,6 +34,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class KeyspaceSteps { @@ -46,7 +47,7 @@ public void connection_create_keyspace(String name) { public void connection_create_keyspaces(List names) { // TODO: This step should be rewritten once we can create keypsaces without opening sessions for (String name : names) { - client.session(name); + client.session(name).close(); } } @@ -71,6 +72,18 @@ public void connection_delete_keyspaces(List names) { } } + @Then("connection delete keyspace(s); throws exception") + public void connection_delete_keyspaces_throws_exception(List names) { + for (String keyspaceName : names) { + try { + client.keyspaces().delete(keyspaceName); + fail(); + } catch (Exception e) { + // successfully failed + } + } + } + @When("connection delete keyspaces in parallel:") public void connection_delete_keyspaces_in_parallel(List names) { assertTrue(THREAD_POOL_SIZE >= names.size()); diff --git a/test/behaviour/connection/transaction/BUILD b/test/behaviour/connection/transaction/BUILD index 314db8e397..f29ea208bd 100644 --- a/test/behaviour/connection/transaction/BUILD +++ b/test/behaviour/connection/transaction/BUILD @@ -34,9 +34,11 @@ java_library( # Internal Repository Dependencies "@graknlabs_common//:common", + "@graknlabs_graql//java:graql", # External Maven Dependencies "@maven//:junit_junit", + "@maven//:org_hamcrest_hamcrest_library", "@maven//:io_cucumber_cucumber_java", ] ) diff --git a/test/behaviour/connection/transaction/TransactionSteps.java b/test/behaviour/connection/transaction/TransactionSteps.java index 34c1613910..fc39e5ff3f 100644 --- a/test/behaviour/connection/transaction/TransactionSteps.java +++ b/test/behaviour/connection/transaction/TransactionSteps.java @@ -20,8 +20,10 @@ package grakn.client.test.behaviour.connection.transaction; import grakn.client.GraknClient; +import graql.lang.Graql; import io.cucumber.java.en.Then; import io.cucumber.java.en.When; +import org.hamcrest.Matchers; import java.util.ArrayList; import java.util.Iterator; @@ -40,7 +42,9 @@ import static grakn.common.util.Collections.list; import static java.util.Objects.isNull; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class TransactionSteps { @@ -65,6 +69,20 @@ public void for_each_session_open_transactions_of_type(List types) { + for (GraknClient.Session session : sessions) { + for (GraknClient.Transaction.Type type : types) { + try { + GraknClient.Transaction transaction = session.transaction(type); + fail(); + } catch (Exception e) { + // successfully threw + } + } + } + } + @Then("for each session, transaction(s) is/are null: {bool}") public void for_each_session_transactions_are_null(boolean isNull) { for_each_session_transactions_are(transaction -> assertEquals(isNull, isNull(transaction))); @@ -166,7 +184,10 @@ private void for_each_session_transactions_in_parallel_are(Consumer futureTransaction : sessionsToTransactionsParallel.get(session)) { - assertions.add(futureTransaction.thenApply(transaction -> { assertion.accept(transaction); return null; })); + assertions.add(futureTransaction.thenApply(transaction -> { + assertion.accept(transaction); + return null; + })); } } CompletableFuture.allOf(assertions.toArray(new CompletableFuture[0])).join(); @@ -222,4 +243,23 @@ private void for_each_session_in_parallel_transactions_in_parallel_are(Consumer< } CompletableFuture.allOf(assertions.toArray(new CompletableFuture[0])).join(); } + + + // ===================================// + // transaction behaviour with queries // + // ===================================// + + @Then("for each transaction, define query; throws exception containing {string}") + public void for_each_transaction_execute_define_throws_exception(String expectedException, String defineQueryStatements) { + for (GraknClient.Session session : sessions) { + for (GraknClient.Transaction transaction : sessionsToTransactions.get(session)) { + try { + transaction.execute(Graql.parse(defineQueryStatements).asDefine()).get(); + fail(); + } catch (Exception e) { + assertThat(e.getMessage(), Matchers.containsString(expectedException)); + } + } + } + } } diff --git a/test/behaviour/graql/GraqlSteps.java b/test/behaviour/graql/GraqlSteps.java index e1dadcfb2d..1931515b73 100644 --- a/test/behaviour/graql/GraqlSteps.java +++ b/test/behaviour/graql/GraqlSteps.java @@ -28,9 +28,7 @@ import grakn.client.answer.Numeric; import grakn.client.concept.Concept; import grakn.client.concept.Rule; -import grakn.client.concept.SchemaConcept; import grakn.client.concept.thing.Attribute; -import grakn.client.concept.thing.Thing; import grakn.client.test.behaviour.connection.ConnectionSteps; import graql.lang.Graql; import graql.lang.pattern.Conjunction; @@ -91,6 +89,13 @@ public void transaction_is_initialised() { assertTrue(tx.isOpen()); } + @Given("transaction is closed and opened without commit") + public void transaction_is_reopened_without_commit() { + tx.close(); + assertFalse(tx.isOpen()); + tx = session.transaction().write(); + } + @Given("the integrity is validated") public void integrity_is_validated(){ diff --git a/test/integration/answer/AnswerIT.java b/test/integration/answer/AnswerIT.java index 31371cf3f3..cf4539a31c 100644 --- a/test/integration/answer/AnswerIT.java +++ b/test/integration/answer/AnswerIT.java @@ -26,21 +26,22 @@ import grakn.common.test.server.GraknProperties; import grakn.common.test.server.GraknSetup; import graql.lang.Graql; +import graql.lang.query.GraqlGet; import graql.lang.query.GraqlInsert; import graql.lang.statement.Variable; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; -import java.io.File; import java.io.IOException; +import java.util.Iterator; import java.util.List; import java.util.concurrent.TimeoutException; import static grakn.client.GraknClient.Transaction.BatchSize.ALL; import static grakn.client.GraknClient.Transaction.Options.batchSize; -import static grakn.client.GraknClient.Transaction.Options.infer; import static grakn.client.GraknClient.Transaction.Options.explain; +import static grakn.client.GraknClient.Transaction.Options.infer; import static graql.lang.Graql.Token.ValueType.STRING; import static graql.lang.Graql.define; import static graql.lang.Graql.insert; @@ -157,21 +158,6 @@ public void asynchronousWriteQueriesAreCompletedWhenTxCommit() { } } - @Test - public void writingInAReadTransactionThrows() { - try (GraknClient.Session session = client.session("test")) { - try (GraknClient.Transaction tx = session.transaction().read()) { - tx.execute(Graql.parse("define newentity sub entity;").asDefine()); - tx.commit(); - fail(); - } catch (Exception ex) { - if (!ex.getMessage().contains("is read only")) { - fail(); - } - } - } - } - @Test public void whenQueryingWithInferenceOn_inferredResultsExist() { try (GraknClient.Session session = client.session("infer_on")) { @@ -319,6 +305,23 @@ public void whenQueryingWithBatchSizeCustom_runsCorrectly() { } } + @Test + public void concurrentReadQueriesReturnResultsConcurrently() { + try (GraknClient.Session session = client.session("concurrent_read")) { + try (GraknClient.Transaction tx = session.transaction().read()) { + GraqlGet query = Graql.match(var("x").sub("thing")).get(); + + Iterator iterator1 = tx.stream(query).get().iterator(); + Iterator iterator2 = tx.stream(query).get().iterator(); + + while (iterator1.hasNext() || iterator2.hasNext()) { + assertEquals(iterator1.next(), iterator2.next()); + assertEquals(iterator1.hasNext(), iterator2.hasNext()); + } + } + } + } + private void setupLotsOfPeople(GraknClient.Session session, int numberOfPeople) { try (GraknClient.Transaction tx = session.transaction().write()) { tx.execute(Graql.parse("define person sub entity;").asDefine());