Skip to content

Commit 47cfce8

Browse files
gaeljwgaeljw
committed
Improve error message in case of optional capture groups
1 parent 9ec2f94 commit 47cfce8

File tree

4 files changed

+139
-18
lines changed

4 files changed

+139
-18
lines changed

‎scala/sources/gen.scala‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,16 @@
33
* Generates the evil looking apply methods in ScalaDsl#StepBody for Function1 to Function22
44
*/
55
for (i <- 1 to 22) {
6-
val ts = (1 to i).map("T" +).mkString(", ")
6+
val ts = (1 to i).map("T".+).mkString(", ")
77
val f = "(" + ts + ") => Any"
88
val p1 = "def apply[" + ts + "](f: " + f + ")"
99
val p2 = "(implicit " + (1 to i).map(n => "m" + n + ":Manifest[T" + n + "]").mkString(", ") + ")"
1010
val register = "\n register(" +(1 to i).map(n => "m" + n).mkString(", ") + ") {\n"
11-
val pf = " case List(" + (1 to i).map("a" + _ + ":AnyRef").mkString(", ") + ") => \n f(" + (1 to i).map(n => "a" + n + ".asInstanceOf[T" + n + "]").mkString(",\n ") + ")"
12-
val closeRegister = "\n }\n}"
11+
val pf = " case List(" + (1 to i).map("a" + _ + ":AnyRef").mkString(", ") + ") => \n f(" + (1 to i).map(n => "a" + n + ".asInstanceOf[T" + n + "]").mkString(",\n ") + ")\n"
12+
val otherwise = " case _ =>\n throw new IncorrectStepDefinitionException()\n"
13+
val closeRegister = " }\n}"
1314

14-
println(p1 + p2 + " = { " + register + pf + closeRegister + "\n")
15+
println(p1 + p2 + ": Unit = { " + register + pf + otherwise + closeRegister + "\n")
1516
}
1617

1718
/*

‎scala/sources/src/main/scala/io/cucumber/scala/IncorrectStepDefinitionException.scala‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package io.cucumber.scala
2+
3+
import io.cucumber.core.exception.CucumberException
4+
5+
object IncorrectStepDefinitionException {
6+
7+
// Allows to use """ in """xxx"""" strings
8+
private val tripleDoubleQuotes = "\"\"\""
9+
10+
val errorMessage: String =
11+
s"""The arguments received doesn't match the step definition.
12+
|This can happen if you are using a regular expression in your step definition with optional capture groups but mandatory parameters.
13+
|
14+
|For instance:
15+
|
16+
| Given($tripleDoubleQuotes^I am logged in(?: as (.+))?$$$tripleDoubleQuotes) { (user: String) =>
17+
| // Some code
18+
| }
19+
|
20+
|For now, the easiest solution is to declare two steps: one with the capture groups, one without.
21+
|If you feel this is not working for you, please manifest yourself on https://github.com/cucumber/cucumber-jvm-scala/issues/3""".stripMargin
22+
23+
}
24+
25+
class IncorrectStepDefinitionException extends CucumberException(IncorrectStepDefinitionException.errorMessage) {
26+
27+
}

‎scala/sources/src/main/scala/io/cucumber/scala/ScalaDsl.scala‎

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,16 +515,20 @@ private[scala] trait StepDsl extends BaseScalaDsl {
515515
def apply(fun: Fun0): Unit = {
516516
register() {
517517
case Nil => fun.f()
518+
case _ =>
519+
throw new IncorrectStepDefinitionException()
518520
}
519521
}
520522

521523
/*
522524
* Generated apply1 to apply22 below
523525
*/
524-
def apply[T1](f: T1 => Any)(implicit m1: Manifest[T1]): Unit = {
526+
def apply[T1](f: (T1) => Any)(implicit m1: Manifest[T1]): Unit = {
525527
register(m1) {
526528
case List(a1: AnyRef) =>
527529
f(a1.asInstanceOf[T1])
530+
case _ =>
531+
throw new IncorrectStepDefinitionException()
528532
}
529533
}
530534

@@ -533,6 +537,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
533537
case List(a1: AnyRef, a2: AnyRef) =>
534538
f(a1.asInstanceOf[T1],
535539
a2.asInstanceOf[T2])
540+
case _ =>
541+
throw new IncorrectStepDefinitionException()
536542
}
537543
}
538544

@@ -542,6 +548,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
542548
f(a1.asInstanceOf[T1],
543549
a2.asInstanceOf[T2],
544550
a3.asInstanceOf[T3])
551+
case _ =>
552+
throw new IncorrectStepDefinitionException()
545553
}
546554
}
547555

@@ -552,6 +560,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
552560
a2.asInstanceOf[T2],
553561
a3.asInstanceOf[T3],
554562
a4.asInstanceOf[T4])
563+
case _ =>
564+
throw new IncorrectStepDefinitionException()
555565
}
556566
}
557567

@@ -563,6 +573,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
563573
a3.asInstanceOf[T3],
564574
a4.asInstanceOf[T4],
565575
a5.asInstanceOf[T5])
576+
case _ =>
577+
throw new IncorrectStepDefinitionException()
566578
}
567579
}
568580

@@ -575,6 +587,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
575587
a4.asInstanceOf[T4],
576588
a5.asInstanceOf[T5],
577589
a6.asInstanceOf[T6])
590+
case _ =>
591+
throw new IncorrectStepDefinitionException()
578592
}
579593
}
580594

@@ -588,6 +602,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
588602
a5.asInstanceOf[T5],
589603
a6.asInstanceOf[T6],
590604
a7.asInstanceOf[T7])
605+
case _ =>
606+
throw new IncorrectStepDefinitionException()
591607
}
592608
}
593609

@@ -602,6 +618,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
602618
a6.asInstanceOf[T6],
603619
a7.asInstanceOf[T7],
604620
a8.asInstanceOf[T8])
621+
case _ =>
622+
throw new IncorrectStepDefinitionException()
605623
}
606624
}
607625

@@ -617,6 +635,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
617635
a7.asInstanceOf[T7],
618636
a8.asInstanceOf[T8],
619637
a9.asInstanceOf[T9])
638+
case _ =>
639+
throw new IncorrectStepDefinitionException()
620640
}
621641
}
622642

@@ -633,6 +653,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
633653
a8.asInstanceOf[T8],
634654
a9.asInstanceOf[T9],
635655
a10.asInstanceOf[T10])
656+
case _ =>
657+
throw new IncorrectStepDefinitionException()
636658
}
637659
}
638660

@@ -650,6 +672,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
650672
a9.asInstanceOf[T9],
651673
a10.asInstanceOf[T10],
652674
a11.asInstanceOf[T11])
675+
case _ =>
676+
throw new IncorrectStepDefinitionException()
653677
}
654678
}
655679

@@ -668,6 +692,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
668692
a10.asInstanceOf[T10],
669693
a11.asInstanceOf[T11],
670694
a12.asInstanceOf[T12])
695+
case _ =>
696+
throw new IncorrectStepDefinitionException()
671697
}
672698
}
673699

@@ -687,6 +713,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
687713
a11.asInstanceOf[T11],
688714
a12.asInstanceOf[T12],
689715
a13.asInstanceOf[T13])
716+
case _ =>
717+
throw new IncorrectStepDefinitionException()
690718
}
691719
}
692720

@@ -707,6 +735,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
707735
a12.asInstanceOf[T12],
708736
a13.asInstanceOf[T13],
709737
a14.asInstanceOf[T14])
738+
case _ =>
739+
throw new IncorrectStepDefinitionException()
710740
}
711741
}
712742

@@ -728,6 +758,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
728758
a13.asInstanceOf[T13],
729759
a14.asInstanceOf[T14],
730760
a15.asInstanceOf[T15])
761+
case _ =>
762+
throw new IncorrectStepDefinitionException()
731763
}
732764
}
733765

@@ -750,6 +782,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
750782
a14.asInstanceOf[T14],
751783
a15.asInstanceOf[T15],
752784
a16.asInstanceOf[T16])
785+
case _ =>
786+
throw new IncorrectStepDefinitionException()
753787
}
754788
}
755789

@@ -773,6 +807,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
773807
a15.asInstanceOf[T15],
774808
a16.asInstanceOf[T16],
775809
a17.asInstanceOf[T17])
810+
case _ =>
811+
throw new IncorrectStepDefinitionException()
776812
}
777813
}
778814

@@ -797,6 +833,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
797833
a16.asInstanceOf[T16],
798834
a17.asInstanceOf[T17],
799835
a18.asInstanceOf[T18])
836+
case _ =>
837+
throw new IncorrectStepDefinitionException()
800838
}
801839
}
802840

@@ -822,6 +860,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
822860
a17.asInstanceOf[T17],
823861
a18.asInstanceOf[T18],
824862
a19.asInstanceOf[T19])
863+
case _ =>
864+
throw new IncorrectStepDefinitionException()
825865
}
826866
}
827867

@@ -848,6 +888,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
848888
a18.asInstanceOf[T18],
849889
a19.asInstanceOf[T19],
850890
a20.asInstanceOf[T20])
891+
case _ =>
892+
throw new IncorrectStepDefinitionException()
851893
}
852894
}
853895

@@ -875,6 +917,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
875917
a19.asInstanceOf[T19],
876918
a20.asInstanceOf[T20],
877919
a21.asInstanceOf[T21])
920+
case _ =>
921+
throw new IncorrectStepDefinitionException()
878922
}
879923
}
880924

@@ -903,6 +947,8 @@ private[scala] trait StepDsl extends BaseScalaDsl {
903947
a20.asInstanceOf[T20],
904948
a21.asInstanceOf[T21],
905949
a22.asInstanceOf[T22])
950+
case _ =>
951+
throw new IncorrectStepDefinitionException()
906952
}
907953
}
908954

‎scala/sources/src/test/scala/io/cucumber/scala/ScalaDslStepsTest.scala‎

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,23 @@ class ScalaDslStepsTest {
7373

7474
val glue = new GlueWithException()
7575

76-
assertClassStepDefinitionThrow(glue.registry.stepDefinitions.head, "io.cucumber.scala.ScalaDslStepsTest$GlueWithException", "ScalaDslStepsTest.scala", 70, Array())
76+
assertClassStepDefinitionThrow(glue.registry.stepDefinitions.head, classOf[PendingException], "io.cucumber.scala.ScalaDslStepsTest$GlueWithException", "ScalaDslStepsTest.scala", 70, Array())
77+
}
78+
79+
// Note: this is a corner case that we should prevent to happen in the future
80+
@Test
81+
def testDefNullParameters(): Unit = {
82+
83+
class Glue extends ScalaDsl with EN {
84+
Given("Something {}") { (str: String) =>
85+
// Nothing
86+
println(str)
87+
}
88+
}
89+
90+
val glue = new Glue()
91+
92+
assertClassStepDefinitionThrow(glue.registry.stepDefinitions.head, classOf[IncorrectStepDefinitionException], "io.cucumber.scala.ScalaDslStepsTest", "ScalaDslStepsTest.scala", 209, Array(null))
7793
}
7894

7995
// -------------------- Test on object --------------------
@@ -91,7 +107,7 @@ class ScalaDslStepsTest {
91107
//@formatter:on
92108
}
93109

94-
assertObjectStepDefinition(Glue.registry.stepDefinitions.head, "Something", "ScalaDslStepsTest.scala:90", Array(), invoked)
110+
assertObjectStepDefinition(Glue.registry.stepDefinitions.head, "Something", "ScalaDslStepsTest.scala:106", Array(), invoked)
95111
}
96112

97113
@Test
@@ -105,7 +121,7 @@ class ScalaDslStepsTest {
105121
}
106122
}
107123

108-
assertObjectStepDefinition(Glue.registry.stepDefinitions.head, "Something", "ScalaDslStepsTest.scala:103", Array(), invoked)
124+
assertObjectStepDefinition(Glue.registry.stepDefinitions.head, "Something", "ScalaDslStepsTest.scala:119", Array(), invoked)
109125
}
110126

111127
@Test
@@ -121,7 +137,7 @@ class ScalaDslStepsTest {
121137
}
122138
}
123139

124-
assertObjectStepDefinition(Glue.registry.stepDefinitions.head, """Oh boy, (\d+) (\s+) cukes""", "ScalaDslStepsTest.scala:118", Array(new java.lang.Integer(5), "green"), thenumber == 5 && thecolour == "green")
140+
assertObjectStepDefinition(Glue.registry.stepDefinitions.head, """Oh boy, (\d+) (\s+) cukes""", "ScalaDslStepsTest.scala:134", Array(new java.lang.Integer(5), "green"), thenumber == 5 && thecolour == "green")
125141
}
126142

127143
@Test
@@ -134,7 +150,21 @@ class ScalaDslStepsTest {
134150
}
135151
}
136152

137-
assertObjectStepDefinitionThrow(GlueWithException.registry.stepDefinitions.head, "io.cucumber.scala.ScalaDslStepsTest$GlueWithException", "ScalaDslStepsTest.scala", 133, Array())
153+
assertObjectStepDefinitionThrow(GlueWithException.registry.stepDefinitions.head, classOf[PendingException], "io.cucumber.scala.ScalaDslStepsTest$GlueWithException", "ScalaDslStepsTest.scala", 149, Array())
154+
}
155+
156+
// Note: this is a corner case that we should prevent to happen in the future
157+
@Test
158+
def testObjectDefNullParameters(): Unit = {
159+
160+
object Glue extends ScalaDsl with EN {
161+
Given("Something {}") { (str: String) =>
162+
// Nothing
163+
println(str)
164+
}
165+
}
166+
167+
assertClassStepDefinitionThrow(Glue.registry.stepDefinitions.head, classOf[IncorrectStepDefinitionException], "io.cucumber.scala.ScalaDslStepsTest", "ScalaDslStepsTest.scala", 209, Array(null))
138168
}
139169

140170
private def assertClassStepDefinition(stepDetails: ScalaStepDetails, pattern: String, location: String, args: Array[AnyRef], check: => Boolean): Unit = {
@@ -152,31 +182,48 @@ class ScalaDslStepsTest {
152182
assertTrue(check)
153183
}
154184

155-
private def assertClassStepDefinitionThrow(stepDetails: ScalaStepDetails, exceptionClassName: String, exceptionFile: String, exceptionLine: Int, args: Array[AnyRef]): Unit = {
156-
assertStepDefinitionThrow(ScalaStepDefinition(stepDetails, true), exceptionClassName, exceptionFile, exceptionLine, args)
185+
private def assertClassStepDefinitionThrow(stepDetails: ScalaStepDetails,
186+
underlyingExceptionClass: Class[_ <: Exception],
187+
exceptionClassName: String,
188+
exceptionFile: String,
189+
exceptionLine: Int,
190+
args: Array[AnyRef]): Unit = {
191+
assertStepDefinitionThrow(ScalaStepDefinition(stepDetails, true), underlyingExceptionClass, exceptionClassName, exceptionFile, exceptionLine, args)
157192
}
158193

159-
private def assertObjectStepDefinitionThrow(stepDetails: ScalaStepDetails, exceptionClassName: String, exceptionFile: String, exceptionLine: Int, args: Array[AnyRef]): Unit = {
160-
assertStepDefinitionThrow(ScalaStepDefinition(stepDetails, false), exceptionClassName, exceptionFile, exceptionLine, args)
194+
private def assertObjectStepDefinitionThrow(stepDetails: ScalaStepDetails,
195+
underlyingExceptionClass: Class[_ <: Exception],
196+
exceptionClassName: String,
197+
exceptionFile: String,
198+
exceptionLine: Int,
199+
args: Array[AnyRef]): Unit = {
200+
assertStepDefinitionThrow(ScalaStepDefinition(stepDetails, false), underlyingExceptionClass, exceptionClassName, exceptionFile, exceptionLine, args)
161201
}
162202

163-
private def assertStepDefinitionThrow(stepDefinition: StepDefinition, exceptionClassName: String, exceptionFile: String, exceptionLine: Int, args: Array[AnyRef]): Unit = {
203+
private def assertStepDefinitionThrow(stepDefinition: StepDefinition,
204+
underlyingExceptionClass: Class[_ <: Exception],
205+
exceptionClassName: String,
206+
exceptionFile: String,
207+
exceptionLine: Int,
208+
args: Array[AnyRef]): Unit = {
164209
val tried = Try(stepDefinition.execute(args))
165210

166211
assertTrue(tried.isFailure)
167212

168213
val ex = tried.failed.get
169214
assertTrue(ex.isInstanceOf[CucumberInvocationTargetException])
170215

171-
val matched = ex.asInstanceOf[CucumberInvocationTargetException]
172-
.getInvocationTargetExceptionCause
216+
val underlying = ex.asInstanceOf[CucumberInvocationTargetException].getInvocationTargetExceptionCause
217+
assertEquals(underlyingExceptionClass, underlying.getClass)
218+
219+
val matched = underlying
173220
.getStackTrace
174221
.filter(stepDefinition.isDefinedAt)
175222
.head
176223

177224
// The result is different between Scala versions, that's why we don't check it precisely
178225
//assertEquals("$anonfun$can_provide_location_of_step$1", matched.getMethodName)
179-
assertTrue(matched.getClassName.contains(exceptionClassName))
226+
assertTrue(s"${matched.getClassName} did not contain $exceptionClassName", matched.getClassName.contains(exceptionClassName))
180227
assertEquals(exceptionFile, matched.getFileName)
181228
assertEquals(exceptionLine, matched.getLineNumber)
182229
}

0 commit comments

Comments
 (0)