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

Fixed bug where -getSnippet in clj.clj received only the step parameter. #650

Merged
merged 1 commit into from
Jan 26, 2014

Conversation

shaolang
Copy link
Contributor

cucumber.runtime.UndefinedStepsTracker actually calls the Backend
instances with 2 parameters--Step and FunctionNameGenerator--but
Clojure's Backend implementation only accepts one.

Instead of testing generation of Clojure Snippet with a test-only
ClojureSnippet instance, ClojureSnippetTest now tests the real
implementation in clj.clj.

Prior this fix, cucumber-clojure throws the following exceptions:

Exception in thread "main" clojure.lang.ArityException: Wrong number of args (3) passed to: clj$-getSnippet
        at clojure.lang.AFn.throwArity(AFn.java:437)
        at clojure.lang.AFn.invoke(AFn.java:47)
        at cucumber.runtime.clj.Backend.getSnippet(Unknown Source)
        at cucumber.runtime.UndefinedStepsTracker.getSnippets(UndefinedStepsTracker.java:32)
        at cucumber.runtime.Runtime.getSnippets(Runtime.java:183)
        at cucumber.runtime.snippets.SummaryPrinter.printSnippets(SummaryPrinter.java:33)
        at cucumber.runtime.snippets.SummaryPrinter.print(SummaryPrinter.java:18)
        at cucumber.runtime.Runtime.printSummary(Runtime.java:125)
        at cucumber.runtime.Runtime.run(Runtime.java:114)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)

cucumber.runtime.UndefinedStepsTracker actually calls the Backend
instances with 2 parameters--Step and FunctionNameGenerator--but
Clojure's Backend implementation only accepts one.

Instead of testing generation of Clojure Snippet with a test-only
ClojureSnippet instance, ClojureSnippetTest now tests the real
implementation in clj.clj.
@brasmusson
Copy link
Contributor

@aslakhellesoy I have checked this PR and think it is OK to merge (and at the same time remove the now obsolete comment about keeping the snippet templates in clj.clj and the removed ClojureSnippet.java in sync).

  • The getSnippet function of the Clojure Backend needs to conform to the signature of the Backend interface in core (even though the FunctionNameGenerator argument is not used, since Clojure step definitions does not have a function name separate from the step definition regexp).
  • Testing the generated snippets in isolation using a copy of the snippet template in ClojureSnippet is faster (~0.3s instead of ~3s), but testing a copy instead of the real thing also has problems (including hiding this bug), so the solution is the PR seems like an improvement to me.

OK?

@aslakhellesoy
Copy link
Contributor

Ok - go ahead and merge!

brasmusson added a commit that referenced this pull request Jan 26, 2014
Also remove obsolete comment from the Clojure backend.
@brasmusson brasmusson merged commit 5e3d2fd into cucumber:master Jan 26, 2014
@brasmusson
Copy link
Contributor

Merged. Thanks for your contribution @shaolang.

@shaolang
Copy link
Contributor Author

@brasmusson You're welcome. :)

@shaolang shaolang deleted the clojure-get-snippet-fix branch March 4, 2014 16:04
@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants