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

Update tck tests #576

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -278,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
Expand All @@ -297,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
Expand Down Expand Up @@ -439,7 +440,13 @@ case class ExpectResult(expectedResult: CypherValueRecords, source: io.cucumber.
}
}

case class ExpectError(errorType: String, phase: String, detail: String, source: io.cucumber.core.gherkin.Step) extends Step {
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 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)) {
Expand All @@ -455,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 &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,8 @@ object CypherValueRecords {
}
}

case class ExecutionFailed(errorType: 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
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

Expand Down Expand Up @@ -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(
Expand All @@ -162,7 +163,78 @@ 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 : ExecutionFailedWithGqlCode) =>
if (error.gqlCode != gqlCode) {
Left(
ScenarioFailedException(
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"))
}

case (ctx, e @ ExpectErrorWithGQLCodeAndMessage(gqlCode, message, _)) =>
ctx.lastResult match {
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}""".stripMargin,
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 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"))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +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 matching: (.+)$"
val expectErrorWithGQLCodeAndMessageR = EXPECT_ERROR_WITH_GQLCODE_AND_MESSAGE.r
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 fooo()
"""
Then an error with GQL code fooCode should be raised
JoaquimGouveia marked this conversation as resolved.
Show resolved Hide resolved

Scenario: Fail with code and message
Given an empty graph
When executing query:
"""
return fooo()
"""
Then an error with GQL code fooCode should be raised with message matching: fooMessage

Scenario: Fail with any type
Given an empty graph
When executing query:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ class FailureWithSideEffectsTckTest extends AnyFunSuite with Assertions with Mat
CypherValueRecords.empty
case ExecQuery =>
hasExecutedQuery = true
ExecutionFailed(ERROR, COMPILE_TIME, ANY)
//TODO: Check which ExecutionFailed class to instantiate
ExecutionFailedWithLegacyDetail(ERROR, COMPILE_TIME, ANY)
}
}

Expand Down
Copy link
Author

Choose a reason for hiding this comment

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

In order to separate ExecutionFailed instantiations had to add extra case where it checks for query containing "fooo()" instead. Feels a bit wrong.

Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,19 @@ 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))
//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 =>
CypherValueRecords.empty
case ControlQuery =>
CypherValueRecords.empty
case ExecQuery if query.contains("foo()") =>
ExecutionFailed(SYNTAX_ERROR, 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)))
Expand All @@ -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", "dummyPhase", "dummyDetail", Some(e))
case Some(e) => ExecutionFailedWithLegacyDetail("dummyType", "dummyPhase", "dummyDetail", Some(e))
case None => base.cypher(query, params, queryType)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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, _) => }
}
Expand All @@ -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}")
}
Expand All @@ -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)
}
Expand All @@ -151,7 +151,6 @@ trait ValidateSteps extends AppendedClues with Matchers with OptionValues with V
case _ =>
}
}

succeed
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ private class FakeGraph implements Graph {
@Override
public Either<ExecutionFailed, CypherValueRecords> cypher(String query, Map<String, CypherValue> 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);
Expand Down