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

Add cypress as e2e tests framework #12210

Merged
merged 119 commits into from Aug 24, 2020
Merged

Add cypress as e2e tests framework #12210

merged 119 commits into from Aug 24, 2020

Conversation

avdev4j
Copy link
Contributor

@avdev4j avdev4j commented Aug 6, 2020

#11961


Please make sure the below checklist is followed for Pull Requests.

When you are still working on the PR, consider converting it to Draft (bellow reviewers) and adding skip-ci label, you can still see CI build result at your branch.

@avdev4j avdev4j closed this Aug 6, 2020
@avdev4j avdev4j reopened this Aug 6, 2020
generators/app/index.js Outdated Show resolved Hide resolved
Copy link
Member

@mshima mshima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should not remove protractor tests and keep both while cypress is not a full replacement.

generators/app/index.js Outdated Show resolved Hide resolved
generators/app/index.js Outdated Show resolved Hide resolved
generators/e2e/index.js Outdated Show resolved Hide resolved
generators/e2e/index.js Outdated Show resolved Hide resolved
generators/client/templates/vue/.eslintignore.ejs Outdated Show resolved Hide resolved
generators/client/templates/vue/.eslintignore.ejs Outdated Show resolved Hide resolved
generators/e2e/index.js Outdated Show resolved Hide resolved
generators/e2e/index.js Outdated Show resolved Hide resolved
@avdev4j
Copy link
Contributor Author

avdev4j commented Aug 7, 2020

I think you should not remove protractor tests and keep both while cypress is not a full replacement.

We are moving all scenario tests from Protractor to Cypress and plan to deprecate Protractor.
We could have both protractor and cypress in CI and decide to deprecate Protractor and no test it anymore.

generators/app/prompts.js Outdated Show resolved Hide resolved
generators/client/files-common.js Outdated Show resolved Hide resolved
generators/client/files-common.js Outdated Show resolved Hide resolved
Copy link
Member

@MathieuAA MathieuAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My biggest issue is the naming convention used for data-cy

Copy link
Contributor

@clementdessoude clementdessoude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, really impressive !

Just an additional general comment: One thing that is troubling me a bit is some coupling between tests, that rely on the state of another test. It is not recommended in Cypress best practices: https://docs.cypress.io/guides/references/best-practices.html#Having-tests-rely-on-the-state-of-previous-tests

generators/cypress/files.js Outdated Show resolved Hide resolved
generators/cypress/files.js Outdated Show resolved Hide resolved
generators/cypress/index.js Outdated Show resolved Hide resolved
generators/cypress/templates/cypress.json.ejs Outdated Show resolved Hide resolved
generators/cypress/files.js Outdated Show resolved Hide resolved
@avdev4j
Copy link
Contributor Author

avdev4j commented Aug 10, 2020

Great work, really impressive !

Just an additional general comment: One thing that is troubling me a bit is some coupling between tests, that rely on the state of another test. It is not recommended in Cypress best practices: https://docs.cypress.io/guides/references/best-practices.html#Having-tests-rely-on-the-state-of-previous-tests

I agree with and that's why I reworked the register spec file. We plan to do the same with other tests, avoid tests to be dependent from others.

If you are ok @clementdessoude you could check the modifications and tell us if you think the modifications are good enough.

@CLAassistant
Copy link

CLAassistant commented Aug 12, 2020

CLA assistant check
All committers have signed the CLA.

@avdev4j avdev4j force-pushed the add-cypress branch 3 times, most recently from b5aa926 to 67db059 Compare August 12, 2020 16:35
@avdev4j
Copy link
Contributor Author

avdev4j commented Aug 13, 2020

We are waiting for #12243 to remove the draft flag. I think all things are done.

@DanielFran
Copy link
Member

@avdev4j I merge the #12243, you can remove the draft :)

- fix field.js
- add faker to all clients package.json if we use cypress
@pascalgrimaud
Copy link
Member

@avdev4j @adilabed @nassimerrahoui :
A big advice here, to progress in this PR is to stop rebase + push force:
avdev4j force-pushed the avdev4j:add-cypress branch from a6ec7ff to a147f0b yesterday

We lost some commits:

We should use git fetch + git merge name/branch, so no work will be lost

@avdev4j avdev4j changed the base branch from master to cypress August 24, 2020 08:11
@avdev4j avdev4j marked this pull request as ready for review August 24, 2020 08:43
@avdev4j
Copy link
Contributor Author

avdev4j commented Aug 24, 2020

As the work on Cypress is more important than exepcted (even if I think we are close to finish it), we merge this PR on a new Cypress branch. Every further modifications would be done from this branch. When we'll decide, the cypress branch will be merged into the master one.

At least we need to:

  • fix the Cassandra/Vue build
  • Remove the last Protractor build from the CI

@avdev4j avdev4j merged commit 6df18ad into jhipster:cypress Aug 24, 2020
Copy link
Member

@mshima mshima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work everyone.
It looks great!

@nassimerrahoui
Copy link
Contributor

We made it 🥇

@avdev4j
Copy link
Contributor Author

avdev4j commented Aug 24, 2020

The last but not least :
#12297

And I hope we could merge it. GG team !

@DanielFran
Copy link
Member

Great. It is now just missing the update to cypress 5

@nassimerrahoui
Copy link
Contributor

nassimerrahoui commented Aug 24, 2020

Great. It is now just missing the update to cypress 5

We forgot it. Done here jhipster#12298

@avdev4j
Copy link
Contributor Author

avdev4j commented Aug 25, 2020

As we struggle a bit with Cypress 5 CI builds, I suggest to merge Cypress 4+ as is and keep working aside on Cypress 5. The goal is to avoid, as much as possible, conflicts between branches.

@avdev4j avdev4j deleted the add-cypress branch August 25, 2020 08:16
@pascalgrimaud pascalgrimaud added this to the 7.0.0 milestone Oct 18, 2020
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.

[Integration Testing] Add cypress to the main generator
10 participants