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

Cucumber Transforms. Should they work this way? #1055

Closed
luke-hill opened this Issue Nov 16, 2016 · 7 comments

Comments

Projects
None yet
2 participants
@luke-hill

luke-hill commented Nov 16, 2016

Summary

Cucumber Transforms don't behave as one would expect. Having searched around for various bits of information, we have a hacky work-around in place here. Which allows us to use them (As they are intended to be), but doesn't allow us to have more complex ones.

Expected Behavior

If you "explicitly call" Transform A. Transform A should be

  1. Searched for in transforms.rb.
    2a) If found Then ran
    2b) If not found, throw an TransformNotFoundError, possibly inheriting from ArgumentError??
  2. If found: The regex should utilised, and then outputted into your variable/s.

Current Behavior

Cucumber searches through the transforms.rb directory. Looking for the first available Regex pattern that passes as true (Completely). This has one or two benefits, but it is outweighed by the fact that if you have similar transforms (As language agnostic applications such as ours have), then it can cause problems.

This is exacerbated when you then apply functions to the transforms, such as basic class changers (to_s, to_i e.t.c.)

Possible Solution

Sadly I'm not huge into my development. So the only proposed solution would be for the Expected Behaviour to work

Steps to Reproduce (for bugs)

In order to re-produce this, you need two Transforms that either match on the same regex, or similar regex. You could have the following two - and you would only ever be able to run CAPTURE_NUM (N.B. The order here matters)

CAPTURE_INT = Transform /^(-?\d+)$/ do |number|
  number.to_i
end

CAPTURE_NUM = Transform /^(-?\d+)$/ do |number|
  number
end

Every time you call either CAPTURE_NUM or CAPTURE_INT you will only ever get CAPTURE_NUM called. This then causes issues if further steps use logic based off the value being an integer (Such as Maths operations)

Context & Motivation

Self explanatory really. It would be nice to start using more indepth regex. And cucumber is really lovely.

Additional info links:

https://www.relishapp.com/cucumber/cucumber/docs/defining-steps/transforms
https://github.com/cucumber/cucumber/wiki/Step-Argument-Transforms

@mattwynne

This comment has been minimized.

Show comment
Hide comment
@mattwynne

mattwynne Sep 4, 2017

Member

@luke-hill unless I've missed something, I think your example has the same regexp patterns for both CAPTURE_INT and CAPTURE_NUM transforms, am I right? In this instance, would it be better for Cucumber to throw an error saying that multiple transforms matched?

Member

mattwynne commented Sep 4, 2017

@luke-hill unless I've missed something, I think your example has the same regexp patterns for both CAPTURE_INT and CAPTURE_NUM transforms, am I right? In this instance, would it be better for Cucumber to throw an error saying that multiple transforms matched?

@mattwynne

This comment has been minimized.

Show comment
Hide comment
@mattwynne

mattwynne Sep 4, 2017

Member

@luke-hill have you had a chance to try the new https://cucumber.io/blog/2017/07/26/announcing-cucumber-expressions feature in the latest 3.0.0.pre.2 release?

Member

mattwynne commented Sep 4, 2017

@luke-hill have you had a chance to try the new https://cucumber.io/blog/2017/07/26/announcing-cucumber-expressions feature in the latest 3.0.0.pre.2 release?

@luke-hill

This comment has been minimized.

Show comment
Hide comment
@luke-hill

luke-hill Sep 5, 2017

Hi Matt. I'll definitely give it a whirl (You released the 3.0.0.pre1 for us to try the retry formatter.

Honest question. Why is there an issue (Or what is happening under the hood). That makes cucumber not force itself to use the expression I type. In my eyes it's a bit misleading. Below is the kind of analogy (Albeit comical I face)

Me to cucumber: I want cucumber to use transform A
Cucumber to me: Do you mean transform B? or A?
Me to cucumber: I mean A, because I typed A... Listen damnit 🗡

luke-hill commented Sep 5, 2017

Hi Matt. I'll definitely give it a whirl (You released the 3.0.0.pre1 for us to try the retry formatter.

Honest question. Why is there an issue (Or what is happening under the hood). That makes cucumber not force itself to use the expression I type. In my eyes it's a bit misleading. Below is the kind of analogy (Albeit comical I face)

Me to cucumber: I want cucumber to use transform A
Cucumber to me: Do you mean transform B? or A?
Me to cucumber: I mean A, because I typed A... Listen damnit 🗡

@stale

This comment has been minimized.

Show comment
Hide comment
@stale

stale bot Nov 8, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

stale bot commented Nov 8, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

@stale stale bot added the Stale label Nov 8, 2017

@luke-hill

This comment has been minimized.

Show comment
Hide comment
@luke-hill

luke-hill Nov 8, 2017

Hi Matt, been reading up on this, and the new parameter types won't fix this issue. As it's doing a fuzzymatch based off the regex which was the issue I mentioned on the 5th Sep.

Thoughts going forwards? Is this something likely to be fixed?

luke-hill commented Nov 8, 2017

Hi Matt, been reading up on this, and the new parameter types won't fix this issue. As it's doing a fuzzymatch based off the regex which was the issue I mentioned on the 5th Sep.

Thoughts going forwards? Is this something likely to be fixed?

@stale stale bot removed the Stale label Nov 8, 2017

@mattwynne

This comment has been minimized.

Show comment
Hide comment
@mattwynne

mattwynne Nov 8, 2017

Member

@aslakhellesoy is probably the best person to help you @luke-hill. I don't have a good handle on the parameter types / transforms stuff at the moment.

Member

mattwynne commented Nov 8, 2017

@aslakhellesoy is probably the best person to help you @luke-hill. I don't have a good handle on the parameter types / transforms stuff at the moment.

@luke-hill

This comment has been minimized.

Show comment
Hide comment
@luke-hill

luke-hill Nov 22, 2017

The majority of use cases for this are remedied by using Transforms. I'll be opening some more granular cases going forward. Which may/may not be a priority.

I'll close this in favour of that.

luke-hill commented Nov 22, 2017

The majority of use cases for this are remedied by using Transforms. I'll be opening some more granular cases going forward. Which may/may not be a priority.

I'll close this in favour of that.

@luke-hill luke-hill closed this Nov 22, 2017

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