cucumber-attic / cuke4duke

Cucumber support for the JVM: Java, Scala, Groovy, Clojure, Ioke, Javascript, Spring, Guice, PicoContainer, WebDriver, Ant and Maven
http://wiki.github.com/aslakhellesoy/cuke4duke
MIT License
256 stars 72 forks source link

Guice fails when trying to instantiate anonymous and non-static inner classes in step classes #33

Closed nightscape closed 15 years ago

nightscape commented 15 years ago

When a step class contains anonymous or inner classes, these classes get compiled to their proper .class files, e.g. BackendSteps$1.class The Guice factory tries to instantiate those classes which is not possible. I fixed this by wrapping the instantiation code in a try ... catch block, so that the Guice factory continues instead of cancelling. The code is in my repository at http://github.com/nightscape/cuke4duke

Best regards Martin

aslakhellesoy commented 15 years ago

I would like to see a test or feature that fails without your fix before adding this.

Also, please respect the coding conventions already in the code - 4 spaces indentation.

And finally, if you fix several unrelated things on your fork, please make a branch for each - that makes it easier for me to merge stuff in. (Your maven stuff doesn't look ready yet).

aslakhellesoy commented 15 years ago

Any progress on adding tests?

nightscape commented 15 years ago

I've been on vacation the last days, need to catch up at work a little, than I will look into writing the corresponding tests. I'll post a reply here on the issue when I'm done.

nightscape commented 15 years ago

I've implemented a testcase, which shows the problem not on a file level (.class files of anonymous and inner classes being picked up) but by directly using addClass on GuiceFactory. Unfortunately, I'm not experienced enough in Git to remove the Maven stuff from the branch of the fix, but the fix only concerns GuiceFactory, GuiceFactoryTest and SomeModule. If you can give me a hint about how I can split off the Maven stuff I will do that as well.

aslakhellesoy commented 15 years ago

Thanks for that. Don't worry about separating the stuff now. I can cherry-pick the commits I want. Here is a rough guide you can follow if you should want to contribute more: http://wiki.github.com/dchelimsky/rspec/topic-branches

(instead of git format-patch master, just do a git push origin my_topic)

nightscape commented 15 years ago

Thanks for developing such a great testing tool! I think I got that with git, next time it'll be a clean fast-forward :) Should I close the ticket or do you close it once you've reviewed the changes?

aslakhellesoy commented 15 years ago

Thanks mate. I'll close the ticket when I have merged in your commits and pushed them to my repo. Hope to get to it within a week, hopefully less. Definitely before next release.

aslakhellesoy commented 15 years ago

Cleaned up indentation and code. Closed by 8158505b2313ca0f9bc9b4383c4421e316932e78

aslakhellesoy commented 15 years ago

With respect to indentation - please use the conventions already present in the code - 4 space indentation.

nightscape commented 15 years ago

Oops, sorry... Just writing some Scala stuff, for which indentation is 2 spaces. Forgot to set it back in Eclipse before formatting...