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

fix(cypress): Merge all steps of a scenario into a single test. #44

Merged
merged 5 commits into from
Apr 19, 2018

Conversation

BenoitAverty
Copy link
Contributor

@BenoitAverty BenoitAverty commented Apr 17, 2018

This is a proof of concept / illustration regardin the issue #43 . It is not meant to be merged right now or as-is, but this issue may force me to abandon cypress so i'd like this proposal to be considered.

I've put a cy.log() command to mark the start of each step, but this is not as good as before wrt reporting. Once cypress-io/cypress#686 is implemented, it should be possible to go back to separate tests by controlling the state clearing within a scenario. In the meantime, I think this is the only way to use gherkin + cypress in my use case (keep state between tests), and it's very likely that other people will run into the same issue.

  • Merge all steps of a scenario into only one test
  • Transform "background" into a beforeEach block
  • Handle Scenario outline
  • Make sure other parts of the module are not broken
  • Update tests

BREAKING CHANGE: each scenario is implemented as a single test now. We lose the ability to know exactly which step fails within a scenario
@lgandecki
Copy link
Collaborator

Thanks @BenoitAverty !
You are completely right, this is a blocker for a lot of use cases. We briefly talked about it here:

#3

I was hoping for the cypress-io/cypress#686 to happen soon. It might.

In: cypress-io/cypress#1473 @brian-mann says
"Lifecycle events. This is a longstanding issue and its fully documented here which should shed light on what's happening under the hood. This is super high up on our radar and will begin immediately after 3.0 release. This is all fixable."

but it's hard to say when is 3.0 supposed to come out.

Meanwhile, it would be great if you could fix the linting issue (./node_modules/.bin/eslint --fix) so we can see what happens with the tests. There is a chance they might work.. all in all if we could make it meanwhile the way you propose, without breaking tests for people that already use this - I think it might be the way to go

@lgandecki
Copy link
Collaborator

Ok, maybe it's going to be sooner or later based on this tweet:

https://twitter.com/bahmutov/status/983848803341099009

@BenoitAverty
Copy link
Contributor Author

Awesome,glad you're considering it. cypress-io/cypress#686 would make it way better, but it looks like a big proposal so I'm not sure it'll come up fast enough.

In any case, i'll fix the lint issue tomorrow and see if I can't tackle the beforeEach task.

Thanks !

@lgandecki
Copy link
Collaborator

Cool. As I thought, the tests just passed which is great - meaning, this would be backward compatible change for the most part. Two things:

  • the ones with background might be false positivies? We need to manually verify them now.
  • the test names seem messed up.. the report becomes rather unusable this way but that should be fixable.

I'm warming up to this approach for the time being if we are able to iron out the remaining weirdnesses

Thanks so much for your help :)

@BenoitAverty
Copy link
Contributor Author

I think I'm pretty much finished with the changes. I diffed the report from jest with the one from master and made sure it is what I expected : the steps have disappeared but the scenario names themselves should be the same.

I have looked around the code and I didn't find other problems but I'll let you do your review.

With this PR, I should be able to use cypress and gherkin to make my QA team happy.

Assuming you decide to merge this, I'll try to find the time to get back the steps failures once cypress-io/cypress#686 is ready.

@BenoitAverty
Copy link
Contributor Author

Here's the diff between the reports before/after
http://www.mergely.com/No4W0kFX/

It has become useless since we don't execute steps as individual tests anymore. It will become useful again when cypress allows to control state clearing and we can get steps as tests again.
@lgandecki lgandecki merged commit 92b5d77 into badeball:master Apr 19, 2018
@lgandecki
Copy link
Collaborator

Thanks again. Looks good. I will update the docs and publish a new, major version tomorrow.
It's sad to see the steps disappear from the reporters, especially for the "examples".. and not being able to see what step failed.. but then having this basically NOT work for any realistic e2e tests out of the box is too much of a problem anyway, and I think this is indeed the right solution for now.

@BenoitAverty BenoitAverty deleted the merge-tests branch April 19, 2018 14:55
@BenoitAverty
Copy link
Contributor Author

Thanks a lot!

@slavede
Copy link

slavede commented Sep 6, 2018

So is this feature now released with the latest version? If so, how can I see this kind of report?

@swatantra15081986
Copy link

@badeball
@lgandecki : what code need to update so that all merge all scenarios with steps in single page ?where we have to put ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants