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

support calling a scenario for data set up of dynamic scenario outlines #1905

Closed
ptrthomas opened this issue Feb 2, 2022 · 26 comments
Closed
Assignees

Comments

@ptrthomas
Copy link
Member

based on discussion in #1903

As of today this works:

Feature:

@ignore @setup
Scenario:
* def data = [{a: 1}, {a: 2}]

Scenario:
* def temp = karate.call('@setup').data
* match temp == [{a: 1}, {a: 2}]

what needs to work is this:

Feature:

@ignore @setup
Scenario:
* def data = [{a: 1}, {a: 2}]

Scenario Outline:
* print __row

Examples:
| karate.call('@setup').data |

right now the karate.call() fails, there is some work to do in sequencing the call, making sure it runs at the time we init the feature etc. cc @edwardsph @joelpramos

@edwardsph
Copy link
Contributor

+1 This is a great solution, thanks.

@ptrthomas
Copy link
Member Author

ptrthomas commented Aug 3, 2022

@edwardsph @joelpramos just realized a complication. what if there is a Background. so here is what I propose:

Feature:

Background:
* print 'in background'

@setup
Scenario:
* def data = [{a: 1}, {a: 2}]

Scenario Outline:
* print __row

Examples:
| karate.setup().data |
  • @setup will be introduced as a "built-in" tag.
  • @setup will not be run when the Feature is run, it behaves like @ignore. it can only be "called"
  • so there is no need to have @ignore, @setup is sufficient
  • there can be only one @setup in a Feature. but as we see later, we can pass arguments to it
  • the Background will run for every Scenario even if it is an Outline row, or generated by a dynamic outline. this is a breaking change
  • the Background will NOT run for the @setup scenario. yes this is a bit inconsistent, but this is the best I could come up with. yes, @setup is "special"
  • we can introduce a karate.setup() API. it can have a second form karate.setup({ argName: 'argValue' })
  • does karate-config.js execute before the @setup ? yes
  • does karate-config.js execute before each row of a dynamic scenario-outline ? this is hard one. I vote yes, for consistency

thoughts !

@ptrthomas
Copy link
Member Author

cc @ericdriggs for thoughts since I remember you have a lot of complex "setup" flows

@ptrthomas
Copy link
Member Author

another advantage of this system, it encourages re-use of data setup functions within any feature. so the very first example in this thread becomes:

Feature:

@setup
Scenario:
* def data = [{a: 1}, {a: 2}]

Scenario:
* def temp = karate.setup().data
* match temp == [{a: 1}, {a: 2}]

@ptrthomas
Copy link
Member Author

ptrthomas commented Aug 5, 2022

another reason why the Background should NOT run for a @setup is clear. because setup() can be called from a Background !

another question can be should this support a callonce mode, my opinion is no, if needed, within the @setup body, the use of callonce or callSingle() is possible

EDIT see #2210

@edwardsph
Copy link
Contributor

That all looks good to me. I wasn't sure about the problems with allowing a callonce mode but I agree that since you can achieve the same thing within the @setup body, then that is a perfectly good solution.

@joelpramos
Copy link
Contributor

My first thought is the limitation of only one setup per feature. Can we change that by, for e.g., if there are multiple force a value name to the tag @setup=scenario1, @setup=scenario2 and throw an error if someone has multiple @setup in a feature or multiple setup with same matching value

suggest the karate.setup() api to receive a string as a parameter and the default value is null (in addition to that second argument)

For ‘callonce’ I think it should. One of the examples I have is the parameters of the test being keys/ids for a UI test but those keys/ids were pulled via an API call to a backend system as the system is a COTS product that auto generates those

Background not running - not a problem. Was thinking here it could be inconvenient at times but there could be a @setup=background some can put in and not use in Examples tables but call directly from the other @setup Examples (assuming you’ll proceed with my suggestion). If it’s just one @setup Scenario a bit of Ctrl C + Ctrl V never hurt anyone

@ptrthomas
Copy link
Member Author

@joelpramos okay, I'll try the @setup=name idea. I am against callonce because I think the workaround mentioned earlier is reasonable, and just to reduce the API learning curve

@joelpramos
Copy link
Contributor

Maybe I misunderstood the above. When you say support callonce mode is that it won’t support scenario outline dynamic (or however we want to call this now !) if the Feature is called from callonce or that the callonce keyword won’t be supported within the @setup method ?

@edwardsph
Copy link
Contributor

To check my own understanding, I think you were saying you will not add a flag to @setup marking it as callonce mode, but rather you will allow callonce to be used within @setup so it is up to the developer if they make a single API call for all scenarios using the setup plus additional code that is run on every setup if they choose. We obviously need to make sure this is clear in docs so I thought it would be useful to see how different people interpreted this.

@ptrthomas
Copy link
Member Author

yes what @edwardsph said. to be honest troubleshooting callonce issues is not fun.

propose I get the basic flow working, then consider

which reminds me @joelpramos maybe we should release 1.3.0.RC1 without this big change, it may make it easier for you and others to validate the JS / multithread fix

@joelpramos
Copy link
Contributor

I’m aligned with the behaviour, it was my misunderstanding.

Yes that would be great, Peter.

ptrthomas added a commit that referenced this issue Aug 6, 2022
ptrthomas added a commit that referenced this issue Aug 7, 2022
remaining - include the extra scenario in reports
documentation, and the support for multiple named setup scenarios in a feature
@ptrthomas
Copy link
Member Author

work in progress, pasting a particularly interesting example of before and after

  • functions can be used at the point of calling karate.setup()
  • user has to choose which variable to "use" out of the setup() but this allows for some flexibility
  • I am putting in the @setup=foo and karate.setup('foo') option which will allow for even more flexibility

part of the breaking change notes: earlier, all variables declared in the Background would be "visible". when we convert the Background to a @setup+Scenario, if we still need some variables to be pre-set for every Scenario just add a Background section.

in other words, the setup() has only one role: generate the scenarios, and that is it. if there is common code for each Scenario, that should be in the Background. and concepts such as callonce are now the same as before

image

ptrthomas added a commit that referenced this issue Aug 7, 2022
multiple setup tags callable by name supported
the setup scenario is also included in the feature report
they even show up in the timeline report in the scenario-iterator thread
only catch is if the setup runs more than once, it will be duplicated in the report
which may be okay to figure how much overhead it adds
@ericdriggs
Copy link

ericdriggs commented Aug 8, 2022

No strong opinion. Calling features from within outlines seems a nice to have. I don't grok why setup is needed in background nor why singleton nature is required.

@joelpramos
Copy link
Contributor

Looking good to me. Based on the description whatever is calculated out of the setup() will also be available on that background if it’s needed for some reason?

I have no further suggestions I think might actually be needed. Above seems to serve the purpose you set out to solve.

A suggest I have is to allow combinations of multiple examples tables in this fashion as well similar to “normal” examples tables (e.g. karate.setup(‘foo’, ‘bar’). Never had the need or use myself or used around as far as I’m aware but just a thought from a parity of capability with non-dynamic scenarios / example tables. Not sure complexity that might or might not introduce.

@ptrthomas
Copy link
Member Author

@joelpramos no, the design is that a setup only returns data. once a outline "row" fires, the variables will be available as __row etc. in the Background - but I need to double-check. I think this avoids a lot of the complexity we have seen in the past with shared scope, call etc

@joelpramos
Copy link
Contributor

I don’t have a strong opinion for the need to have it available and like I said before, some copy pasting never hurt nobody if there’s really a need and there are multiple scenarios in the same Feature file. Might just need to be a detail mentioned in the docs.

@ptrthomas
Copy link
Member Author

@joelpramos yes, I still need to write the docs for this and change existing docs.

one thing I was able to validate, see screenshot below. so even in cases where we had a Background doing "callonce" kind of stuff, the new pattern works fine with only the "data" moving to the extra "setup" Scenario. so I think all cases are covered. I really wanted to avoid contributing variables in a "pre background" phase, which would open up questions on call vs callonce. we can certainly consider or revisit in future.

image

@edwardsph
Copy link
Contributor

I'm interested in how this affects the reports. I currently generate a report for each test that includes the background and scenario steps. I suspect the setup steps will not be included since they are only used to generate the test. Is that correct?

@ptrthomas
Copy link
Member Author

@edwardsph the good news is that the @setup will appear in the report, but look like a regular Scenario and not "inline" where it was called from. this can be improved in future, but I thought it is a reasonable start

@ptrthomas ptrthomas mentioned this issue Aug 22, 2022
@ptrthomas
Copy link
Member Author

attention all - this change was merged to develop and is now available in maven central as 1.3.0.RC2

@ptrthomas
Copy link
Member Author

1.3.0 released

@almohankumar
Copy link

@ptrthomas This is great. This adds to the readability and re-usability.

@ptrthomas
Copy link
Member Author

attention all - decided to add an option to run the setup once, refer #2210

@tuguldur-dev
Copy link

Hello @ptrthomas, Thanks for creating great tool.
I have a little problem about in Scenario Outline. Send whole row as a request. Is it possible?
I also tried bunch of times. But getting error like
image

@ptrthomas
Copy link
Member Author

@dududadadodo please use stack overflow or follow this process: https://github.com/karatelabs/karate/wiki/How-to-Submit-an-Issue

@karatelabs karatelabs locked as resolved and limited conversation to collaborators Oct 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants