From 5512a1ebed2159ea15b52c1e5a958d6ea945064a Mon Sep 17 00:00:00 2001 From: Joaquim Date: Fri, 28 Jun 2024 16:05:28 +0200 Subject: [PATCH 1/4] added initial support for gql status codes --- .../opencypher/tools/tck/api/CypherTCK.scala | 9 +++- .../org/opencypher/tools/tck/api/Result.scala | 2 +- .../opencypher/tools/tck/api/Scenario.scala | 45 +++++++++++++++++++ .../tck/constants/TCKStepDefinitions.scala | 5 +++ .../org/opencypher/tools/tck/Foo.feature | 16 +++++++ .../tck/FailureWithSideEffectsTckTest.scala | 2 +- .../org/opencypher/tools/tck/TckTest.scala | 6 +-- .../cucumber/CucumberReportAdapterTest.java | 2 +- 8 files changed, 80 insertions(+), 7 deletions(-) diff --git a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/CypherTCK.scala b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/CypherTCK.scala index 40fcc2c9e..b0dd6d3b6 100644 --- a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/CypherTCK.scala +++ b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/CypherTCK.scala @@ -260,7 +260,8 @@ object CypherTCK { } } List(expectedError) - + case expectErrorWithGQLCodeR(gqlCode) => List(ExpectErrorWithGQLCode(gqlCode, step)) + case expectErrorWithGQLCodeAndMessageR(gqlCode, message) => List(ExpectErrorWithGQLCodeAndMessage(gqlCode, message, step)) // And case noSideEffectsR() => List(SideEffects(source = step).fillInZeros) case sideEffectsR() => List(SideEffects(parseSideEffectsTable, step).fillInZeros) @@ -439,6 +440,12 @@ case class ExpectResult(expectedResult: CypherValueRecords, source: io.cucumber. } } +case class ExpectErrorWithGQLCode(gqlCode: String, source: io.cucumber.core.gherkin.Step) extends Step { +} + +case class ExpectErrorWithGQLCodeAndMessage(gqlCode: String, message: String, source: io.cucumber.core.gherkin.Step) extends Step { +} + case class ExpectError(errorType: String, phase: String, detail: String, source: io.cucumber.core.gherkin.Step) extends Step { // Returns None if valid and Some("error message") otherwise. def validate(): Option[String] = { diff --git a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Result.scala b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Result.scala index 9064793ac..62f5ee921 100644 --- a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Result.scala +++ b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Result.scala @@ -82,4 +82,4 @@ object CypherValueRecords { } } -case class ExecutionFailed(errorType: String, phase: String, detail: String, exception: Option[Throwable] = None) +case class ExecutionFailed(errorType: String, gqlCode: String, message: String, phase: String, detail: String, exception: Option[Throwable] = None) diff --git a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala index 6209c6915..c110e2034 100644 --- a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala +++ b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala @@ -167,6 +167,51 @@ case class Scenario(categories: List[String], featureName: String, number: Optio Left(ScenarioFailedException(s"Expected: $e, got records $records")) } + case (ctx, e @ ExpectErrorWithGQLCode(gqlCode, _)) => + ctx.lastResult match { + case Left(error) => + if (error.gqlCode != gqlCode) { + Left( + ScenarioFailedException( + s"Wrong error code: expected $gqlCode, got ${error.gqlCode}", + error.exception.orNull)) + } else { + Right(ctx) + } + case Right(records) => + Left(ScenarioFailedException(s"Expected: $e, got records $records")) + } + + case (ctx, e @ ExpectErrorWithGQLCodeAndMessage(gqlCode, message, _)) => + ctx.lastResult match { + case Left(error) => + val isSameCode = error.gqlCode == gqlCode + val isSameMessage = error.message == message + if (!isSameCode && !isSameMessage) { + Left( + ScenarioFailedException( + s"Wrong GQL error code and message: expected GQL code: $gqlCode, got ${error.gqlCode}, expected message: $message, got ${error.message}" + ) + ) + } else if (!isSameCode) { + Left( + ScenarioFailedException( + s"Wrong GQL error code: expected GQL code: $gqlCode, got ${error.gqlCode}" + ) + ) + } else if (!isSameMessage) { + Left( + ScenarioFailedException( + s"Wrong error message: expected message: $message, got ${error.message}" + ) + ) + } else { + Right(ctx) + } + case Right(records) => + Left(ScenarioFailedException(s"Expected: $e, got records $records")) + } + case (ctx, SideEffects(expected, _)) => val before = ctx.state val after = ctx.measure.state diff --git a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/constants/TCKStepDefinitions.scala b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/constants/TCKStepDefinitions.scala index 7bde5c9d6..d243ad0d4 100644 --- a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/constants/TCKStepDefinitions.scala +++ b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/constants/TCKStepDefinitions.scala @@ -88,4 +88,9 @@ object TCKStepDefinitions { val EXPECT_ERROR = "^an? (.+) should be raised at (.+): (.+)$" val expectErrorR = EXPECT_ERROR.r + val EXPECT_ERROR_WITH_GQLCODE = "^an? error with GQL code (.+) should be raised$" + val expectErrorWithGQLCodeR = EXPECT_ERROR_WITH_GQLCODE.r + + val EXPECT_ERROR_WITH_GQLCODE_AND_MESSAGE = "^an? error with GQL code (.+) should be raised with message (.+)$" + val expectErrorWithGQLCodeAndMessageR = EXPECT_ERROR_WITH_GQLCODE_AND_MESSAGE.r } diff --git a/tools/tck-api/src/test/resources/org/opencypher/tools/tck/Foo.feature b/tools/tck-api/src/test/resources/org/opencypher/tools/tck/Foo.feature index 0cd10ef77..83ea6dd3c 100644 --- a/tools/tck-api/src/test/resources/org/opencypher/tools/tck/Foo.feature +++ b/tools/tck-api/src/test/resources/org/opencypher/tools/tck/Foo.feature @@ -67,6 +67,22 @@ Feature: Foo """ Then a SyntaxError should be raised at compile time: UnknownFunction + Scenario: Fail with code + Given an empty graph + When executing query: + """ + RETURN foo() + """ + Then an error with GQL code fooCode should be raised + + Scenario: Fail with code and message + Given an empty graph + When executing query: + """ + return foo() + """ + Then an error with GQL code fooCode should be raised with message fooMessage + Scenario: Fail with any type Given an empty graph When executing query: diff --git a/tools/tck-api/src/test/scala/org/opencypher/tools/tck/FailureWithSideEffectsTckTest.scala b/tools/tck-api/src/test/scala/org/opencypher/tools/tck/FailureWithSideEffectsTckTest.scala index 59ef82b9d..57c8f15c8 100644 --- a/tools/tck-api/src/test/scala/org/opencypher/tools/tck/FailureWithSideEffectsTckTest.scala +++ b/tools/tck-api/src/test/scala/org/opencypher/tools/tck/FailureWithSideEffectsTckTest.scala @@ -91,7 +91,7 @@ class FailureWithSideEffectsTckTest extends AnyFunSuite with Assertions with Mat CypherValueRecords.empty case ExecQuery => hasExecutedQuery = true - ExecutionFailed(ERROR, COMPILE_TIME, ANY) + ExecutionFailed(ERROR, null, null, COMPILE_TIME, ANY) } } diff --git a/tools/tck-api/src/test/scala/org/opencypher/tools/tck/TckTest.scala b/tools/tck-api/src/test/scala/org/opencypher/tools/tck/TckTest.scala index 66089ffe4..1d5885ee6 100644 --- a/tools/tck-api/src/test/scala/org/opencypher/tools/tck/TckTest.scala +++ b/tools/tck-api/src/test/scala/org/opencypher/tools/tck/TckTest.scala @@ -96,7 +96,7 @@ class TckTest extends AnyFunSpec with Assertions with Matchers { override def cypher(query: String, params: Map[String, CypherValue], queryType: QueryType): Result = { queryType match { case InitQuery if query.contains("FAIL") => - ExecutionFailed(SYNTAX_ERROR, COMPILE_TIME, "fail", Some(FAIL_EXCEPTION)) + ExecutionFailed(SYNTAX_ERROR, null, null, COMPILE_TIME, "fail", Some(FAIL_EXCEPTION)) case InitQuery if !query.contains("FAIL") => CypherValueRecords.empty case SideEffectQuery => @@ -104,7 +104,7 @@ class TckTest extends AnyFunSpec with Assertions with Matchers { case ControlQuery => CypherValueRecords.empty case ExecQuery if query.contains("foo()") => - ExecutionFailed(SYNTAX_ERROR, COMPILE_TIME, UNKNOWN_FUNCTION) + ExecutionFailed(SYNTAX_ERROR, "fooCode", "fooMessage", COMPILE_TIME, UNKNOWN_FUNCTION) // assert that csv path parameter is not overwritten by additional parameters case ExecQuery if query.contains("LOAD CSV") && params.keySet.equals(Set("param", "list")) => StringRecords(List("res"), cvsData.rows.map(r => Map("res" -> r("txt").toString))) @@ -125,7 +125,7 @@ class TckTest extends AnyFunSpec with Assertions with Matchers { private case class FailingGraph(base: Graph)(failureFor: PartialFunction[QueryType, Throwable]) extends Graph with ProcedureSupport { override def cypher(query: String, params: Map[String, CypherValue], queryType: QueryType): Result = { failureFor.lift.apply(queryType) match { - case Some(e) => ExecutionFailed("dummyType", "dummyPhase", "dummyDetail", Some(e)) + case Some(e) => ExecutionFailed("dummyType", "dummyCode", "dummyMessage", "dummyPhase", "dummyDetail", Some(e)) case None => base.cypher(query, params, queryType) } } diff --git a/tools/tck-reporting/src/test/java/org/opencypher/tools/tck/reporting/cucumber/CucumberReportAdapterTest.java b/tools/tck-reporting/src/test/java/org/opencypher/tools/tck/reporting/cucumber/CucumberReportAdapterTest.java index ab5997afc..b5eaec8ca 100644 --- a/tools/tck-reporting/src/test/java/org/opencypher/tools/tck/reporting/cucumber/CucumberReportAdapterTest.java +++ b/tools/tck-reporting/src/test/java/org/opencypher/tools/tck/reporting/cucumber/CucumberReportAdapterTest.java @@ -81,7 +81,7 @@ private class FakeGraph implements Graph { @Override public Either cypher(String query, Map params, QueryType meta) { if (query.contains("foo()")) { - return new Left<>(new ExecutionFailed("SyntaxError", "compile time", "UnknownFunction", null)); + return new Left<>(new ExecutionFailed("SyntaxError", null, null,"compile time", "UnknownFunction", null)); } else if (query.startsWith("RETURN ")) { String result = query.replace("RETURN ", ""); System.out.println("Producing some output " + result); From 81e0b9293955c740ffe6ffc6d72451c4a190b293 Mon Sep 17 00:00:00 2001 From: Joaquim Date: Mon, 1 Jul 2024 16:13:50 +0200 Subject: [PATCH 2/4] Refractoring --- .../opencypher/tools/tck/api/Scenario.scala | 41 +++++++++---------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala index c110e2034..32cbc55c8 100644 --- a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala +++ b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala @@ -179,7 +179,8 @@ case class Scenario(categories: List[String], featureName: String, number: Optio Right(ctx) } case Right(records) => - Left(ScenarioFailedException(s"Expected: $e, got records $records")) + Left( + ScenarioFailedException(s"Expected: $e, got records $records")) } case (ctx, e @ ExpectErrorWithGQLCodeAndMessage(gqlCode, message, _)) => @@ -187,26 +188,24 @@ case class Scenario(categories: List[String], featureName: String, number: Optio case Left(error) => val isSameCode = error.gqlCode == gqlCode val isSameMessage = error.message == message - if (!isSameCode && !isSameMessage) { - Left( - ScenarioFailedException( - s"Wrong GQL error code and message: expected GQL code: $gqlCode, got ${error.gqlCode}, expected message: $message, got ${error.message}" - ) - ) - } else if (!isSameCode) { - Left( - ScenarioFailedException( - s"Wrong GQL error code: expected GQL code: $gqlCode, got ${error.gqlCode}" - ) - ) - } else if (!isSameMessage) { - Left( - ScenarioFailedException( - s"Wrong error message: expected message: $message, got ${error.message}" - ) - ) - } else { - Right(ctx) + (isSameCode, isSameMessage) match { + case (false, false) => + Left( + ScenarioFailedException( + s"Wrong GQL error code and message: expected GQL code: $gqlCode, got ${error.gqlCode}, expected message: $message, got ${error.message}", + error.exception.orNull)) + case (false, true) => + Left( + ScenarioFailedException( + s"Wrong GQL error code: expected GQL code: $gqlCode, got ${error.gqlCode}", + error.exception.orNull)) + case (true, false) => + Left( + ScenarioFailedException( + s"Wrong error message: expected message: $message, got ${error.message}", + error.exception.orNull)) + case (true, true) => + Right(ctx) } case Right(records) => Left(ScenarioFailedException(s"Expected: $e, got records $records")) From bfa145a22fcd8b6e3764bc59ca58ea853a29b05b Mon Sep 17 00:00:00 2001 From: Joaquim Date: Fri, 5 Jul 2024 13:24:10 +0200 Subject: [PATCH 3/4] Separation of old and new error tests. Improved test messages --- .../opencypher/tools/tck/api/CypherTCK.scala | 20 ++++---- .../org/opencypher/tools/tck/api/Result.scala | 6 ++- .../opencypher/tools/tck/api/Scenario.scala | 46 +++++++++++++++---- .../tck/constants/TCKStepDefinitions.scala | 6 +-- .../org/opencypher/tools/tck/Foo.feature | 6 +-- .../tck/FailureWithSideEffectsTckTest.scala | 3 +- .../org/opencypher/tools/tck/TckTest.scala | 10 ++-- .../opencypher/tools/tck/ValidateSteps.scala | 13 +++--- 8 files changed, 73 insertions(+), 37 deletions(-) diff --git a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/CypherTCK.scala b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/CypherTCK.scala index b0dd6d3b6..8faf1fd4e 100644 --- a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/CypherTCK.scala +++ b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/CypherTCK.scala @@ -247,8 +247,8 @@ object CypherTCK { case expectSortedResultR() => List(ExpectResult(parseTable(), step, sorted = true)) case expectResultUnorderedListsR() => List(ExpectResult(parseTable(orderedLists = false), step)) case expectSortedResultUnorderedListsR() => List(ExpectResult(parseTable(orderedLists = false), step, sorted = true)) - case expectErrorR(errorType, time, detail) => - val expectedError = ExpectError(errorType, time, detail, step) + case expectErrorWithLegacyDetailR(errorType, time, detail) => + val expectedError = ExpectErrorWithLegacyDetail(errorType, time, detail, step) if (shouldValidate) { expectedError.validate() match { case None => // No problem @@ -279,13 +279,13 @@ object CypherTCK { val (name, number) = parseNameAndNumber(nameAndNumber) val tagsInferred = tags ++ Set(TCKTags.NEGATIVE_TEST, TCKTags.WILDCARD_ERROR_DETAILS).filter { case TCKTags.NEGATIVE_TEST => transformedSteps.exists { - case _: ExpectError => true + case _: ExpectErrorWithLegacyDetail => true case _ => false } case TCKTags.WILDCARD_ERROR_DETAILS => transformedSteps.exists { - case ExpectError(TCKErrorTypes.ERROR, _, _, _) => true - case ExpectError(_, TCKErrorPhases.ANY_TIME, _, _) => true - case ExpectError(_, _, TCKErrorDetails.ANY, _) => true + case ExpectErrorWithLegacyDetail(TCKErrorTypes.ERROR, _, _, _) => true + case ExpectErrorWithLegacyDetail(_, TCKErrorPhases.ANY_TIME, _, _) => true + case ExpectErrorWithLegacyDetail(_, _, TCKErrorDetails.ANY, _) => true case _ => false } case _ => false @@ -298,8 +298,8 @@ object CypherTCK { private def insertSideEffectsOnExpectError(originalSteps: List[Step]): List[Step] = { @tailrec def recurse(steps: List[Step], done: ListBuffer[Step]): List[Step] = steps match { - case (_: ExpectError) :: (_: SideEffects) :: _ => originalSteps // We already have side effects - case (expectError: ExpectError) :: tail => + case (_: ExpectErrorWithLegacyDetail) :: (_: SideEffects) :: _ => originalSteps // We already have side effects + case (expectError: ExpectErrorWithLegacyDetail) :: tail => // Insert empty side effects after expect error val sideEffects = SideEffects(source = expectError.source).fillInZeros (done ++= (expectError :: sideEffects :: tail)).toList @@ -446,7 +446,7 @@ case class ExpectErrorWithGQLCode(gqlCode: String, source: io.cucumber.core.gher case class ExpectErrorWithGQLCodeAndMessage(gqlCode: String, message: String, source: io.cucumber.core.gherkin.Step) extends Step { } -case class ExpectError(errorType: String, phase: String, detail: String, source: io.cucumber.core.gherkin.Step) extends Step { +case class ExpectErrorWithLegacyDetail(errorType: String, phase: String, detail: String, source: io.cucumber.core.gherkin.Step) extends Step { // Returns None if valid and Some("error message") otherwise. def validate(): Option[String] = { if (!TCKErrorTypes.ALL.contains(errorType)) { @@ -462,7 +462,7 @@ case class ExpectError(errorType: String, phase: String, detail: String, source: override def equals(obj: Any): Boolean = { obj match { - case ExpectError(thatErrorType, thatPhase, thatDetail, thatSource) => + case ExpectErrorWithLegacyDetail(thatErrorType, thatPhase, thatDetail, thatSource) => thatErrorType == errorType && thatPhase == phase && thatDetail == detail && diff --git a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Result.scala b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Result.scala index 62f5ee921..88092a98a 100644 --- a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Result.scala +++ b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Result.scala @@ -82,4 +82,8 @@ object CypherValueRecords { } } -case class ExecutionFailed(errorType: String, gqlCode: String, message: String, phase: String, detail: String, exception: Option[Throwable] = None) +sealed trait ExecutionFailed { + def exception: Option[Throwable] +} +case class ExecutionFailedWithLegacyDetail(errorType: String, phase: String, detail: String, exception: Option[Throwable] = None) extends ExecutionFailed +case class ExecutionFailedWithGqlCode(gqlCode: String, message: String, exception: Option[Throwable] = None) extends ExecutionFailed \ No newline at end of file diff --git a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala index 32cbc55c8..2a4ffc980 100644 --- a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala +++ b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala @@ -97,7 +97,8 @@ case class Scenario(categories: List[String], featureName: String, number: Optio case ControlQuery => Right(execResult) case InitQuery => execResult.lastResult match { case Right(_) => Right(execResult) - case Left(error) => Left(ScenarioFailedException(s"Got error $error", error.exception.orNull)) + case Left(error : ExecutionFailed) => + Left(ScenarioFailedException(s"Got error $error", error.exception.orNull)) } } @@ -137,13 +138,13 @@ case class Scenario(categories: List[String], featureName: String, number: Optio } else { Right(ctx) } - case Left(error) => + case Left(error : ExecutionFailed) => Left(ScenarioFailedException(s"Expected: $expected, got error $error", error.exception.orNull)) } - case (ctx, e @ ExpectError(errorType, phase, detail, _)) => + case (ctx, e @ ExpectErrorWithLegacyDetail(errorType, phase, detail, _)) => ctx.lastResult match { - case Left(error) => + case Left(error : ExecutionFailedWithLegacyDetail) => if (error.errorType != errorType && error.errorType != TCKErrorTypes.ERROR && errorType != TCKErrorTypes.ERROR) Left( ScenarioFailedException( @@ -162,22 +163,39 @@ case class Scenario(categories: List[String], featureName: String, number: Optio else { Right(ctx) } - + case Left(error : ExecutionFailedWithGqlCode) => + Left( + ScenarioFailedException( + s"""Expected legacy detail error with: + |error type: $errorType, + |error phase: $phase, + |error detail: $detail + |But got error with GQL code: ${error.gqlCode}""".stripMargin + ) + ) case Right(records) => Left(ScenarioFailedException(s"Expected: $e, got records $records")) } case (ctx, e @ ExpectErrorWithGQLCode(gqlCode, _)) => ctx.lastResult match { - case Left(error) => + case Left(error : ExecutionFailedWithGqlCode) => if (error.gqlCode != gqlCode) { Left( ScenarioFailedException( - s"Wrong error code: expected $gqlCode, got ${error.gqlCode}", + s"Wrong GQL error code: expected $gqlCode, got ${error.gqlCode}", error.exception.orNull)) } else { Right(ctx) } + case Left(error : ExecutionFailedWithLegacyDetail) => + Left( + ScenarioFailedException( + s"""Expected error with GQL code: $gqlCode + |But got error with legacy detail + |""".stripMargin + ) + ) case Right(records) => Left( ScenarioFailedException(s"Expected: $e, got records $records")) @@ -185,14 +203,16 @@ case class Scenario(categories: List[String], featureName: String, number: Optio case (ctx, e @ ExpectErrorWithGQLCodeAndMessage(gqlCode, message, _)) => ctx.lastResult match { - case Left(error) => + case Left(error : ExecutionFailedWithGqlCode) => val isSameCode = error.gqlCode == gqlCode val isSameMessage = error.message == message (isSameCode, isSameMessage) match { case (false, false) => Left( ScenarioFailedException( - s"Wrong GQL error code and message: expected GQL code: $gqlCode, got ${error.gqlCode}, expected message: $message, got ${error.message}", + s"""Wrong GQL error code and message: + | expected GQL code: $gqlCode, got ${error.gqlCode}, + | expected message: $message, got ${error.message}""".stripMargin, error.exception.orNull)) case (false, true) => Left( @@ -207,6 +227,14 @@ case class Scenario(categories: List[String], featureName: String, number: Optio case (true, true) => Right(ctx) } + case Left(error : ExecutionFailedWithLegacyDetail) => + Left( + ScenarioFailedException( + s"""Expected error with GQL code: $gqlCode and message: $message + |But got error with legacy detail + |""".stripMargin + ) + ) case Right(records) => Left(ScenarioFailedException(s"Expected: $e, got records $records")) } diff --git a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/constants/TCKStepDefinitions.scala b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/constants/TCKStepDefinitions.scala index d243ad0d4..dfb2fe8f9 100644 --- a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/constants/TCKStepDefinitions.scala +++ b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/constants/TCKStepDefinitions.scala @@ -85,12 +85,12 @@ object TCKStepDefinitions { val EXPECT_EMPTY_RESULT = "^the result should be empty$" val expectEmptyResultR = EXPECT_EMPTY_RESULT.r - val EXPECT_ERROR = "^an? (.+) should be raised at (.+): (.+)$" - val expectErrorR = EXPECT_ERROR.r + val EXPECT_ERROR_WITH_LEGACY_DETAIL = "^an? (.+) should be raised at (.+): (.+)$" + val expectErrorWithLegacyDetailR = EXPECT_ERROR_WITH_LEGACY_DETAIL.r val EXPECT_ERROR_WITH_GQLCODE = "^an? error with GQL code (.+) should be raised$" val expectErrorWithGQLCodeR = EXPECT_ERROR_WITH_GQLCODE.r - val EXPECT_ERROR_WITH_GQLCODE_AND_MESSAGE = "^an? error with GQL code (.+) should be raised with message (.+)$" + val EXPECT_ERROR_WITH_GQLCODE_AND_MESSAGE = "^an? error with GQL code (.+) should be raised with message matching: (.+)$" val expectErrorWithGQLCodeAndMessageR = EXPECT_ERROR_WITH_GQLCODE_AND_MESSAGE.r } diff --git a/tools/tck-api/src/test/resources/org/opencypher/tools/tck/Foo.feature b/tools/tck-api/src/test/resources/org/opencypher/tools/tck/Foo.feature index 83ea6dd3c..34a5b0fd1 100644 --- a/tools/tck-api/src/test/resources/org/opencypher/tools/tck/Foo.feature +++ b/tools/tck-api/src/test/resources/org/opencypher/tools/tck/Foo.feature @@ -71,7 +71,7 @@ Feature: Foo Given an empty graph When executing query: """ - RETURN foo() + RETURN fooo() """ Then an error with GQL code fooCode should be raised @@ -79,9 +79,9 @@ Feature: Foo Given an empty graph When executing query: """ - return foo() + return fooo() """ - Then an error with GQL code fooCode should be raised with message fooMessage + Then an error with GQL code fooCode should be raised with message matching: fooMessage Scenario: Fail with any type Given an empty graph diff --git a/tools/tck-api/src/test/scala/org/opencypher/tools/tck/FailureWithSideEffectsTckTest.scala b/tools/tck-api/src/test/scala/org/opencypher/tools/tck/FailureWithSideEffectsTckTest.scala index 57c8f15c8..14c981dff 100644 --- a/tools/tck-api/src/test/scala/org/opencypher/tools/tck/FailureWithSideEffectsTckTest.scala +++ b/tools/tck-api/src/test/scala/org/opencypher/tools/tck/FailureWithSideEffectsTckTest.scala @@ -91,7 +91,8 @@ class FailureWithSideEffectsTckTest extends AnyFunSuite with Assertions with Mat CypherValueRecords.empty case ExecQuery => hasExecutedQuery = true - ExecutionFailed(ERROR, null, null, COMPILE_TIME, ANY) + //TODO: Check which ExecutionFailed class to instantiate + ExecutionFailedWithLegacyDetail(ERROR, COMPILE_TIME, ANY) } } diff --git a/tools/tck-api/src/test/scala/org/opencypher/tools/tck/TckTest.scala b/tools/tck-api/src/test/scala/org/opencypher/tools/tck/TckTest.scala index 1d5885ee6..2bff35d7b 100644 --- a/tools/tck-api/src/test/scala/org/opencypher/tools/tck/TckTest.scala +++ b/tools/tck-api/src/test/scala/org/opencypher/tools/tck/TckTest.scala @@ -96,7 +96,8 @@ class TckTest extends AnyFunSpec with Assertions with Matchers { override def cypher(query: String, params: Map[String, CypherValue], queryType: QueryType): Result = { queryType match { case InitQuery if query.contains("FAIL") => - ExecutionFailed(SYNTAX_ERROR, null, null, COMPILE_TIME, "fail", Some(FAIL_EXCEPTION)) + //TODO: Check which ExecutionFailed class to instantiate + ExecutionFailedWithLegacyDetail(SYNTAX_ERROR, COMPILE_TIME, "fail", Some(FAIL_EXCEPTION)) case InitQuery if !query.contains("FAIL") => CypherValueRecords.empty case SideEffectQuery => @@ -104,7 +105,10 @@ class TckTest extends AnyFunSpec with Assertions with Matchers { case ControlQuery => CypherValueRecords.empty case ExecQuery if query.contains("foo()") => - ExecutionFailed(SYNTAX_ERROR, "fooCode", "fooMessage", COMPILE_TIME, UNKNOWN_FUNCTION) + //TODO: Check which ExecutionFailed class to instantiate + ExecutionFailedWithLegacyDetail(SYNTAX_ERROR, COMPILE_TIME, UNKNOWN_FUNCTION) + case ExecQuery if query.contains("fooo()") => + ExecutionFailedWithGqlCode("fooCode", "fooMessage") // assert that csv path parameter is not overwritten by additional parameters case ExecQuery if query.contains("LOAD CSV") && params.keySet.equals(Set("param", "list")) => StringRecords(List("res"), cvsData.rows.map(r => Map("res" -> r("txt").toString))) @@ -125,7 +129,7 @@ class TckTest extends AnyFunSpec with Assertions with Matchers { private case class FailingGraph(base: Graph)(failureFor: PartialFunction[QueryType, Throwable]) extends Graph with ProcedureSupport { override def cypher(query: String, params: Map[String, CypherValue], queryType: QueryType): Result = { failureFor.lift.apply(queryType) match { - case Some(e) => ExecutionFailed("dummyType", "dummyCode", "dummyMessage", "dummyPhase", "dummyDetail", Some(e)) + case Some(e) => ExecutionFailedWithLegacyDetail("dummyType", "dummyPhase", "dummyDetail", Some(e)) case None => base.cypher(query, params, queryType) } } diff --git a/tools/tck-integrity-tests/src/test/scala/org/opencypher/tools/tck/ValidateSteps.scala b/tools/tck-integrity-tests/src/test/scala/org/opencypher/tools/tck/ValidateSteps.scala index 63b06cc3b..2e4f527a2 100644 --- a/tools/tck-integrity-tests/src/test/scala/org/opencypher/tools/tck/ValidateSteps.scala +++ b/tools/tck-integrity-tests/src/test/scala/org/opencypher/tools/tck/ValidateSteps.scala @@ -32,7 +32,7 @@ import org.opencypher.tools.tck.api.ControlQuery import org.opencypher.tools.tck.api.CsvFile import org.opencypher.tools.tck.api.ExecQuery import org.opencypher.tools.tck.api.Execute -import org.opencypher.tools.tck.api.ExpectError +import org.opencypher.tools.tck.api.ExpectErrorWithLegacyDetail import org.opencypher.tools.tck.api.ExpectResult import org.opencypher.tools.tck.api.InitQuery import org.opencypher.tools.tck.api.Parameters @@ -63,7 +63,7 @@ trait ValidateSteps extends AppendedClues with Matchers with OptionValues with V case (Execute(_, ExecQuery, _), ix) => numberOfExecQuerySteps += 1 positionFirstExecQuery = Math.min(positionFirstExecQuery, ix) case (Execute(_, ControlQuery, _), _) => numberOfControlQuerySteps += 1 - case (_: ExpectError, _) => numberOfExpectErrorSteps += 1 + case (_: ExpectErrorWithLegacyDetail, _) => numberOfExpectErrorSteps += 1 case (_: ExpectResult, _) => numberOfExpectResultSteps += 1 case (se: SideEffects, _) if se.source.getType == StepType.AND => numberOfExplicitSideEffectSteps += 1 case (se: SideEffects, _) if se.source.getType == StepType.THEN => numberOfImplicitSideEffectSteps += 1 @@ -99,7 +99,7 @@ trait ValidateSteps extends AppendedClues with Matchers with OptionValues with V withClue(s"${er.description} is preceded by a `When executing query` or `When executing control query` step") { predecessor should matchPattern { case Execute(_, ExecQuery | ControlQuery, _) => } } - case (predecessor, ee: ExpectError) => + case (predecessor, ee: ExpectErrorWithLegacyDetail) => withClue(s"${ee.description} is preceded by a `When executing query` step") { predecessor should matchPattern { case Execute(_, ExecQuery, _) => } } @@ -109,9 +109,9 @@ trait ValidateSteps extends AppendedClues with Matchers with OptionValues with V } case (predecessor, se: SideEffects) if se.source.getType == StepType.THEN => withClue(s"${se.description} is preceded by a `Then expect error` step") { - predecessor should matchPattern { case _: ExpectError => } + predecessor should matchPattern { case _: ExpectErrorWithLegacyDetail => } } - case (ee: ExpectError, successor) => + case (ee: ExpectErrorWithLegacyDetail, successor) => withClue(s"${ee.description} is not succeeded by anything, i.e. is the last step") { fail(s"${ee.description} is succeeded by ${successor.description}") } @@ -125,7 +125,7 @@ trait ValidateSteps extends AppendedClues with Matchers with OptionValues with V steps foreach { case e: Execute => validateQuery(e, tags) case se: SideEffects => validateSideEffects(se) - case ee: ExpectError => + case ee: ExpectErrorWithLegacyDetail => withClue(s"${ee.description} has valid type") { TCKErrorTypes.ALL should contain(ee.errorType) } @@ -151,7 +151,6 @@ trait ValidateSteps extends AppendedClues with Matchers with OptionValues with V case _ => } } - succeed } } From 300e78daddb9624d675b10a5d44445df61974165 Mon Sep 17 00:00:00 2001 From: Joaquim Date: Fri, 5 Jul 2024 13:27:42 +0200 Subject: [PATCH 4/4] Message update --- .../src/main/scala/org/opencypher/tools/tck/api/Scenario.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala index 2a4ffc980..53d825cac 100644 --- a/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala +++ b/tools/tck-api/src/main/scala/org/opencypher/tools/tck/api/Scenario.scala @@ -183,7 +183,7 @@ case class Scenario(categories: List[String], featureName: String, number: Optio if (error.gqlCode != gqlCode) { Left( ScenarioFailedException( - s"Wrong GQL error code: expected $gqlCode, got ${error.gqlCode}", + s"Wrong GQL error code: expected GQL code $gqlCode, got ${error.gqlCode}", error.exception.orNull)) } else { Right(ctx)