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

catch all transformation functionality #1209

Closed
akostadinov opened this issue Oct 9, 2017 · 55 comments
Closed

catch all transformation functionality #1209

akostadinov opened this issue Oct 9, 2017 · 55 comments
Labels
⌛ stale Will soon be closed by stalebot unless there is activity

Comments

@akostadinov
Copy link
Contributor

Hello, as of #1203 the Transform was removed. We heavily use a catch all transformation which also transforms table cells. At present it seems transforming cells is not supported in any way.

Can we have a catch all transformation method in 3.0 (all non-empty parameters and table cells)? At present I don't see us able to upgrade to 3.0 ever if we can't do catch-all transformations.

@aslakhellesoy
Copy link
Contributor

I'm not aware that Transform would transform table cells in data tables or Scenario Outline/Example tables.

If you provide a miminal example in a git repo that uses this old feature I'll do my best to show how to do the same with ParameterType. -Or suggest a change to Cucumber to make the transition easier.

@akostadinov
Copy link
Contributor Author

I was talking about table cells of steps:

Scenario: check radar
  ...
  Given the radar detects:
    | ships | 5  |
    | boats | 10 |
  ...

I can craft a small example a little later but thought to clarify what I mean first. Thanks for reply.

@akostadinov
Copy link
Contributor Author

Sorry for delay, I created a minimal cucumber project that hopefully demonstrates one important use case we have [1].

I think that any method that allows us to just process all parameters (strings or tables) would do the job. So if you know any way to catch all params, we can replace our usage of Transform with it.

[1] https://github.com/akostadinov/testcucumber_transform/blob/master/features/users.feature

@eheinen
Copy link

eheinen commented Oct 13, 2017

I have the same issue in my features, but it is pretty simple, like:
| attribute | value |
| min_accepted | 60 |

When I have a table with a title combination like attribute and value, then my transform get this combination by Regex and format it according my code bellow:

Transform /^table:attribute,value$/ do |table|
table.map_headers! { |header| header.downcase.to_sym }
table
end

@aslakhellesoy
Copy link
Contributor

Please make it easy for us to investigate this by providing a minimal example in a git repo. I won't try to cobble together an example by copy-pasting code from comments.

@akostadinov
Copy link
Contributor Author

@aslakhellesoy , in my previous comment I provided such a git repo. I'm not sure whether your comment was a reply to @eheinen or you missed or didn't like my example repo.

@eheinen
Copy link

eheinen commented Oct 13, 2017

It was for me I guess, don't worries. I am creating the example now

@aslakhellesoy
Copy link
Contributor

@akostadinov sorry I missed it! Thanks a lot - will have a look.

@eheinen
Copy link

eheinen commented Oct 13, 2017

@aslakhellesoy I published my example. It is running:
https://github.com/eheinen/minimal-example-cucumber-3-table-format/blob/master/features/minimal.feature
If you think something is missing, just let me know please.
Thanks

@aslakhellesoy
Copy link
Contributor

aslakhellesoy commented Oct 13, 2017

Thanks - much appreciated. I see it was added in f0e85dc (version 0.4.0) 8 years ago :-) I never used that feature myself, and admit I completely missed its existence when I replaced Transform with DataTable. Sorry about that.

I'm afraid the new ParameterType doesn't support that (yet), but we do have plans for it. See cucumber/common#210.

@akostadinov
Copy link
Contributor Author

What I see in cucumber/common#210 is the idea to transform table based on headers (unless I'm missing something).

This is not actually useful for many use cases. Can we just have a "catch-all" transformer in the event one wants a more customized solution. For our use case transforming into objects is done on a case by case basis depending on the step. For this we use different methods in a custom World class to achieve DRY.

The transformation we would like to have is to handle some low-level generic things (like in the example). Would there be any drawbacks to also have "catch-all" transformer that just processes all parameters (in addition to the neat new param transformer methods)?

@stale
Copy link

stale bot commented Dec 12, 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 Will soon be closed by stalebot unless there is activity label Dec 12, 2017
@akostadinov
Copy link
Contributor Author

Not sure the intention of this aggressive bug closing. If I'm waiting on project owners feedback am I supposed to just bump the topic?

@stale stale bot removed the ⌛ stale Will soon be closed by stalebot unless there is activity label Dec 12, 2017
@aslakhellesoy
Copy link
Contributor

The frequency of new issues being created is significantly higher that the frequency of existing issues being fixed.

The reason for this is simple - only a handful of people contribute on a regular basis (without getting paid for it) while the vast majority of users (over 99%) just use the software and file issues without contributing.

Over the years, this led to a steady increase of open issues (people were creating them faster than we could close them), and at the beginning of 2017 there were about 500 open issues across the cucumber GitHub organisation. That was frustrating for everyone, users and contributors alike.

A couple of months ago we adopted a policy (aided by stalebot) where issues without any activity for 3 months get marked as stale, and if there is no further activity, it gets closed. Today, the total number of open issues is down to about 1/4 of that.

By closing issues like this, we're being honest: It doesn't look like this is going to get done. I prefer to be honest rather than lulling you into a false hope that it's going to get fixed one day.

This helps the core team focus on what's important. Focussing is incredibly hard when there are too many open issues.

It also sends a clear signal to our user base that open source software isn't free. Somebody paid for it (with their time). If you're not able to, or willing to, invest your own time or money to make a contribution, then your issue probably won't get fixed.

The core team prioritises issues based on how big of a problem it is perceived to be. This issue doesn't appear to be a big enough problem enough people, and nobody seems to care enough about it to do the work to fix it.

That's the way open source works. I hope I've answered your question.

If you have any suggestions about how to improve the situation I'm all ears.

@aslakhellesoy
Copy link
Contributor

If I'm waiting on project owners feedback am I supposed to just bump the topic?

Yes, you are. This sends us a signal that you still care about this.

@jones-agyemang
Copy link

@aslakhellesoy I appreciate you for all your good work on the Cucumber estate.

@akostadinov
Copy link
Contributor Author

wrt improving situation - I don't think there is a silver bullet and arguments go both ways (about closing inactive bugs). IMO a better triage process will help. e.g. depending on bug type an inactivity close may or may not make so much sense. I understand though that proper triaging is hardly achievable even in commercial organizations. So we are back to square one :)

Personally my biggest hurdle to contributing code is when there is some decision to be made - it is usually hard to understand what kind of solution if any would be accepted. Like with this issue. I would look at implementing it if I knew whether feature would be accepted and how.
Basically any non-trivial contribution takes a lot of effort to get through. Especially when contributions are occasional and I imagine there are only a few people that can focus on a single project long term.

As an example for a very successful project for me is Jenkins. Since I monitored it from the beginning, I saw Kohsuke hand-hold new contributors to get them started. Then aggressively made a lot of extension points so that core can be small and the rest of the functionality could be implemented by plugins that don't need to be vetted by the core team. In that project though he was payed full time on it AFAIK. Which brings us back to your point (about getting payed).

In any case, it is a two way process - how project handles contributions is very important for attracting contributors.

P.S. I'm highlighting "code" above, because contributing bug reports is also contribution. I know many people that can't be bugged to report them. And even I (a great enthusiast for filing bugs) have stopped reporting bugs that I see ignored from upstream projects because this also requires effort and time (not talking about Cucumber but other projects).

@Githraine
Copy link

I dont know if this is related, but I am having a lot of issues with Scenario Outlines, and have need no mention of them in any of the docs for this update. Has anyone seen them work under 3.0?

@phebus
Copy link

phebus commented Feb 19, 2018

Just chiming in to say I am also very much interested in a solution to this as we rely heavily on being able to replace data in data tables based on regexes. I will research as much as possible to see if I can contribute a solution, but in the meantime just wanted to +1 this feature.

@xtrasimplicity
Copy link
Member

I've never used Transforms, but based on the following they look interesting.

https://github.com/cucumber/cucumber/wiki/Step-Argument-Transforms

Could someone please write/append some feature definitions and steps, to further illustrate this? I'd be happy to take a look at how we could implement this, when I get a chance.

@akostadinov
Copy link
Contributor Author

@xtrasimplicity , please check my comment #1209 (comment)

There is an example how it could work in the past. IMO for 3.x it is ok to have a "catch all" transformation in addition to the new available transformation options to allow all use cases. I mean we don't need restoring the full regex based transformation in 2.x series.

@stale
Copy link

stale bot commented Apr 21, 2018

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 Will soon be closed by stalebot unless there is activity label Apr 21, 2018
@xtrasimplicity
Copy link
Member

Bump. I'll get to this one day... :)

@stale stale bot removed the ⌛ stale Will soon be closed by stalebot unless there is activity label Apr 21, 2018
@stale
Copy link

stale bot commented Jun 20, 2018

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 Will soon be closed by stalebot unless there is activity label Jun 20, 2018
@akostadinov
Copy link
Contributor Author

unstale

@xtrasimplicity xtrasimplicity removed the ⌛ stale Will soon be closed by stalebot unless there is activity label Mar 2, 2019
@akostadinov
Copy link
Contributor Author

bump

@sky-eduardolopes
Copy link

We have been delaying upgrading cucumber to 3.x for a while because of this. Are there any plans to add this capability?

@luke-hill
Copy link
Contributor

No committers / maintainers have jumped on this. However this is the kind of thing that would benefit from re-linking to aslak's comment above

#1209 (comment)

And the opencollective link: https://opencollective.com/cucumber

@akostadinov
Copy link
Contributor Author

@luke-hill , it is not only about writing some support code. When project owners need to advise what is acceptable and with what approach, there can hardly be contributions.

@luke-hill
Copy link
Contributor

There is a framework issue feature request created here - cucumber/common#210

If you worked against that spec I would review as I'm sure others would, and it would be pushed in.

@akostadinov
Copy link
Contributor Author

@luke-hill , I think cucumber/common#210 is about figuring out table implementation. Question here is whether catch-all would be accepted or not. Does it make sense now or am I missing something?

@luke-hill
Copy link
Contributor

I think your best bet given the length of this thread is to produce a small spike for how it would work in one example and then go from there,

Github now allows you to post a WIP PR, so you could potentially do that?

@stale
Copy link

stale bot commented May 25, 2019

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 Will soon be closed by stalebot unless there is activity label May 25, 2019
@andyw8
Copy link

andyw8 commented May 25, 2019

It's not ideal, but I worked around this with a method which parses table.hashes to convert some common types:

module AutoTransform
  MONEY = /\$([\d\.\,]+)/.freeze
  DATE = /(\d\d\d\d-\d\d-\d\d)/.freeze
  PERCENTAGE = /([\d\.]+)\%/.freeze

  def auto_transform(table_hashes)
    table_hashes.map do |hash|
      hash.transform_values do |value|
        if (value_match = value.match(MONEY))
          BigDecimal(value_match[1].delete(','))
        elsif (value_match = value.match(DATE))
          Date.parse(value_match[1])
        elsif (value_match = value.match(PERCENTAGE))
          BigDecimal(value_match[1])
        else
          value
        end
      end
    end
  end
end

Then within the step definitions file, I add World(AutoTransform) to the end, and wrap the call to the table, e.g. auto_transform(table.hashes).each do...

@stale stale bot removed the ⌛ stale Will soon be closed by stalebot unless there is activity label May 25, 2019
@luke-hill
Copy link
Contributor

@andyw8 is this something you feel would add value to the core codebase. Do you feel you could add enough wrapping around it (Comments / specs e.t.c.) to get a PR ready>?

@andyw8
Copy link

andyw8 commented May 28, 2019

I don't think it's really suitable for that, I would prefer to see this solved properly in cucumber/common#210.

@stale
Copy link

stale bot commented Jul 27, 2019

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 Will soon be closed by stalebot unless there is activity label Jul 27, 2019
@xtrasimplicity
Copy link
Member

Bump

@stale stale bot removed the ⌛ stale Will soon be closed by stalebot unless there is activity label Jul 27, 2019
@luke-hill
Copy link
Contributor

@andyw8 / @akostadinov do we see value in keeping this open alongside 210 above? Or closing this in favour of that one?

@stale
Copy link

stale bot commented Sep 27, 2019

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 Will soon be closed by stalebot unless there is activity label Sep 27, 2019
@akostadinov
Copy link
Contributor Author

@luke-hill , please see my comment #1209 (comment) .

@stale stale bot removed the ⌛ stale Will soon be closed by stalebot unless there is activity label Sep 28, 2019
@luke-hill
Copy link
Contributor

Yep, I've seen all the comments, I'm asking if this issue request is worth keeping open? Is it something you're likely to work on, or you think should be worked on? If so why?

@akostadinov
Copy link
Contributor Author

Would such feature be accepted in the project? I have given an example usage in #1209 (comment) .

@luke-hill
Copy link
Contributor

I will happily look into any PR's, given this was something which was an old feature and then no longer used.

Obviously given the specificity of it, I would ideally like it to have good test coverage. At the bare minimum I'd expect 100% unit tests and at least 1 feature test.

@stale
Copy link

stale bot commented Dec 7, 2019

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 Will soon be closed by stalebot unless there is activity label Dec 7, 2019
@stale
Copy link

stale bot commented Dec 14, 2019

This issue has been automatically closed because of inactivity. You can support the Cucumber core team on opencollective.

@stale stale bot closed this as completed Dec 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⌛ stale Will soon be closed by stalebot unless there is activity
Projects
None yet
Development

No branches or pull requests

10 participants