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

ci(cypress): add cypress dashboard #2880

Merged
merged 1 commit into from
May 12, 2020
Merged

ci(cypress): add cypress dashboard #2880

merged 1 commit into from
May 12, 2020

Conversation

DlgSHi
Copy link
Contributor

@DlgSHi DlgSHi commented Apr 17, 2020

Proposed behaviour

We've got approved OSS cypress plan for carbon so we need to:

  • update all npm scripts;
  • update .travis.yml file to exclude cypress tests from it;
  • create a cypress-parallel.yml in .github/workflows/ to run all regression using Github Actions CI;
  • delete all deprecated cypress tests;
  • delete all docker / TeamCity setup;
  • clear up the package.json file script section;
  • update the README.md files for cypress and carbon.

Important

  • When the PR will be created from a carbon fork - the GitHub Actions will not pass the cypress record key for that PR that's why the tests will run for 2,5 hours on 1 thread and will not block other runs.

After implementation the code the time for parallel tests for all regression test suite is approx ~10-12 min.
dashboard link
Screenshot 2020-05-04 at 13 19 32

Current behaviour

We are running all our cypress test on one thread. All regression suite took for a approx ~3h.

Checklist

- [ ] Release notes (Conventional Commits)
- [ ] Unit tests

  • Cypress automation tests

- [ ] Storybook added or updated
- [ ] Theme support
- [ ] Typescript d.ts file added or updated

Additional context

Testing instructions

all suites run on Travis using two new script commands:

  • npm run cypress:ci:smoke
  • npm run cypress:ci:all

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 39fd00af9195271f4c5f7554ce2e4a409f75bf14:

Sandbox Source
strange-moon-i9x5n Configuration

@DlgSHi DlgSHi marked this pull request as ready for review April 17, 2020 14:04
@DlgSHi DlgSHi requested review from a team as code owners April 17, 2020 14:04
@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 17, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 17ef8a6:

Sandbox Source
long-dew-z7sb0 Configuration

@DlgSHi DlgSHi force-pushed the cypress-oss branch 2 times, most recently from 1288b15 to ee846ee Compare April 21, 2020 08:54
@DlgSHi DlgSHi force-pushed the cypress-oss branch 5 times, most recently from ae17060 to 870808f Compare April 22, 2020 10:22
Copy link

@jamime jamime left a comment

Choose a reason for hiding this comment

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

Please remove changes related to Team City.

Let's keep this change to Travis only, then we can do a separate PR for CodeShip.

@DlgSHi DlgSHi requested a review from jamime April 23, 2020 09:30
@jamime jamime marked this pull request as draft April 23, 2020 18:54
@jamime
Copy link

jamime commented Apr 23, 2020

Converted to draft to prevent merging while @DlgSHi works on Code Ship.

@DlgSHi
Copy link
Contributor Author

DlgSHi commented May 5, 2020

  • What is the behaviour for PR's from forked repos, will the tests still run?
  • Is it safe to pass secrets.GITHUB_TOKEN to builds on third party repos?

@jamime thanks for your questions,

  1. Yes, it is possible to run Actions on fork - as explained on GitHub, but:
  • Actions needs to be enabled for a fork.
  • cypress record key will not be passed to forked repo and regression will take as it took previously 2,5h and will not be recorded / run in parallel.
  1. Yes it is, for every fork repo access for a secrets is only state on read - as explained here

* Only build suite tests, which are running on Travis after every change/commit/push in the repository. To run use:`npm run test-cypress-build`;
* Only accessibility suite tests, which are running on TeamCity nightly and verifying do the components have the accessibility vulnerabilities. To run use:`npm run test-cypress-accessibility`;
* Regression test suite runs on TeamCity nightly and makes all regression tests. To use run: `npm run test-cypress-regression`.
6. Run cypress `npx cypress open` or `npm run test-cypress`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Write a description of what happens after running these commands, and how to run one specific test file on local

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@@ -483,3 +457,19 @@ When('I click onto root in Test directory in iFrame', () => {
Then('totalRecords is set to {string} {word}', (totalRecords, element) => {
pagerSummary().invoke('text').should('contain', `${totalRecords} ${element}`);
});

Then('{string} component is visible', (component) => {
getComponentNoIframe(component).should('exist');
Copy link
Contributor

Choose a reason for hiding this comment

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

should be visible or exist ? Please change the name of step or assertion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

deleted

});

Then('{string} element is visible', (element) => {
getElementNoIframe(element).should('exist');
Copy link
Contributor

Choose a reason for hiding this comment

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

should be visible or exist ? Please change the name of step or assertion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

deleted

});

Then('{string} element is visible by name', (element) => {
getElementNoIframeByName(element).should('exist');
Copy link
Contributor

Choose a reason for hiding this comment

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

should be visible or exist ? Please change the name of step or assertion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

deleted

@jamime
Copy link

jamime commented May 5, 2020

  • What is the behaviour for PR's from forked repos, will the tests still run?
  • Is it safe to pass secrets.GITHUB_TOKEN to builds on third party repos?

@jamime thanks for your questions,

  1. Yes, it is possible to run Actions on fork - as explained on GitHub, but:
  • Actions needs to be enabled for a fork.
  • cypress record key will not be passed to forked repo and regression will take as it took previously 2,5h and will not be recorded / run in parallel.
  1. Yes it is, for every fork repo access for a secrets is only state on read - as explained here

Ah yes, in that link I see that

When you create a pull request from a forked repository to the base repository, GitHub sends the pull_request event to the base repository and no pull request events occur on the forked repository.

So the fork won't run the build, but there should be an action run from our repo which is what we want 👍

Happy with the permissions too.

It's a shame about the time taken. We have two options

  • Grant users permission to create their own branch e.g. as a collaborator
  • Merge the PR into a branch and we merge it from there, including fixing any cypress tests

For now I think its acceptable that PR's take 2.5 hours to run, the user should be running the tests locally first. It's unlikely that we will review it within 2.5 hours of submitting the PR anyway.

@DlgSHi DlgSHi requested a review from a team as a code owner May 5, 2020 11:29
@DlgSHi DlgSHi requested review from jamime and KatarzynaQA May 5, 2020 11:34
@DlgSHi DlgSHi force-pushed the cypress-oss branch 2 times, most recently from 2094767 to d4f708e Compare May 6, 2020 12:48
@DlgSHi DlgSHi requested a review from jamime May 6, 2020 16:46
@chrisbarber86
Copy link
Member

chrisbarber86 commented May 7, 2020

@jamime If someone does PR from a fork - you can push their branch/commit to github.com/sage/carbon as a manual step and that will run CI on it our side but will update the PR status to reflect it as the commit hash is the same. So don't need to grant contributor access or anything

@jamime
Copy link

jamime commented May 7, 2020

@DlgSHi @chrisbarber86

When the PR will be created from a carbon fork - the GitHub Actions will not pass the cypress record key for that PR that's why the tests will run for 2,5 hours (blocking other runs by putting all other PR in queue

If we already know that PR's from forks are going to run in one thread, can we make it so we don't spawn as many runners. Maybe detect if the record key has been set, I don't think it's possible to detect a fork pr event.

@jamime jamime changed the title chore(cypress): add project config ci(cypress): add cypress dashboard May 7, 2020
@DlgSHi DlgSHi force-pushed the cypress-oss branch 2 times, most recently from 43fd66a to c1bacea Compare May 11, 2020 06:16
KatarzynaQA
KatarzynaQA previously approved these changes May 11, 2020
@jamime
Copy link

jamime commented May 11, 2020

I raised some issues

We have been able to make it run on 1 runner on a fork PR and 20 runners on a regular PR. We still have a little inefficiency when 19 runners might be waiting for the last one to complete, but we can't fix this at this time.

jamime
jamime previously approved these changes May 11, 2020
@DlgSHi DlgSHi dismissed stale reviews from jamime and KatarzynaQA via 17ef8a6 May 11, 2020 14:26
@DlgSHi DlgSHi merged commit 12cee6b into master May 12, 2020
@DlgSHi DlgSHi deleted the cypress-oss branch May 12, 2020 05:26
@carbonci
Copy link
Collaborator

🎉 This PR is included in version 20.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

6 participants