Fix scala #154

Closed
aslakhellesoy opened this Issue Jan 21, 2012 · 11 comments

Comments

Projects
None yet
3 participants
Owner

aslakhellesoy commented Jan 21, 2012

APIs were refactored again, so the code broke since neither of us know enough Scala to fix it.

Owner

aslakhellesoy commented Jan 21, 2012

@dwskoog - is this something you can help us fix?

Contributor

dwskoog commented Jan 21, 2012

Working on it now. Btw, the cucumber-java pom has a deep hate for Windows thanks to that antrun task. Oh how I wish I had something other than my windows laptop right now...

Contributor

dwskoog commented Jan 21, 2012

Okay, my Groovy-fu is a bit weak, but for posterity: /{$basedir}\subdirectory\another/ is how you make a string literal safe for ant-run's groovy task under windows... which of course breaks it horrible on every OS without retarded file separators. I'll make sure not to taint the check-in with that evil.

@teigen teigen added a commit to teigen/cucumber-jvm that referenced this issue Jan 21, 2012

@teigen teigen #154, RunCukesTest.scala uses ScalaBackend.loadGlue which calls Class…
…pathResourceLoader().instantiateSubclasses which didn't check if the class had a valid constructor given the arguments. ScalaDslTest.scala holds multiple internal ScalaDsl instances which are compiled to classes implementing ScalaDsl but with no valid constructor. These were being picked up by the ClasspathResourceLoader causing the RunCukesTest to fail at load time
4eaf0d4
Contributor

dwskoog commented Jan 21, 2012

@teigen is dead on with his answer. The problem is that nested objects/classes don't have nullary constructors because of how Scala handles the scoping details at the jvm level.

There is still a bit of an issue about how the ScalaDsl derivatives are loaded though. It's fairly odd from a Scala POV to have the StepDefs in a class, but the current loading scheme requires it since we're reflecting into public nullary constructors. Given an object Example extends ScalaDsl that would generate a class Example$ which only has a private nullary constructor so it blows up. The right way to get your hands on it is via Example$.MODULE$

@aslakhellesoy aslakhellesoy added a commit that referenced this issue Jan 21, 2012

@aslakhellesoy aslakhellesoy Merge pull request #155 from teigen/fix-154
#154. Fixed the issue causing scala to break
34814b4
Owner

aslakhellesoy commented Jan 21, 2012

@dwskoog are you saying there might be a more idiomatic way to declare step definitions in Scala? Something more similar to Ruby/Groovy/JavaScript/Python?

If so, would you be interested in adding support for it?

Contributor

teigen commented Jan 21, 2012

I wouldn't say its odd to to have step defs in classes, but I agree its odd not to allow them in objects.
Another fun project would be to allow step definitions in scala scripts which would let you organise step definition just like the ruby cucumber api

Contributor

dwskoog commented Jan 21, 2012

@aslakhellesoy It's fairly weird to me that class SomeStepDefs extends ScalaDsl ... works fine but object SomeStepDefs extends ScalaDsl ... spits out the suggestions about how to implement the steps. The snippet that is printed on the console looks exactly like what would be in the object as far as the Given-When-Then lines go. Let me see if I can clear that up sensibly.

The whole issue about making the DSL truly fluent is way out of scope for the 1.0 release. Best to get some explicit feedback from the scala community to figure out where to take that issue.

Contributor

dwskoog commented Jan 21, 2012

@teigen I get a feeling of impedance mismatch between the declarative feel of the DSL and having steps in classes. When I write out a Given(..) {...} for example, to me that's akin to a routing statement and nothing that makes real sense in terms needing an instance of something.

Contributor

teigen commented Jan 21, 2012

@dwskoog the classes are instances in a world.
For step definitions that manage mutable state this is quite important.
You also need isolation if things are to run in parallel which is now the default for test execution in sbt.

But like I said, I completely agree that object should be supported and the current behavior with objects is very far from what one would expect

Contributor

dwskoog commented Jan 21, 2012

@teigen ah, I see why now. Definitely better than the gross reflection abuse you'd need to pull off to hide all of that away from end users.

Owner

aslakhellesoy commented Jan 21, 2012

This is fixed on master - thanks @teigen. The object DSL discussion can continue in #156.

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