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

feat(storybook): cypress setup for the project #282

Merged
merged 10 commits into from
Mar 17, 2021
Merged

feat(storybook): cypress setup for the project #282

merged 10 commits into from
Mar 17, 2021

Conversation

sanketshevkar
Copy link
Member

@sanketshevkar sanketshevkar commented Mar 5, 2021

Signed-off-by: User [email protected]

Changes

  • Added cypress as a devDependency and wrote a dummy test within the cypress/intergrations folder.
  • You need to install cypress on your local machine so as to run tests. Instructions for installing cypress on local machine: https://docs.cypress.io/guides/getting-started/installing-cypress.html#System-requirements
  • To run the test you need two terminal windows, on the first one you need to run npm run storybook and once the storybook loads, on the second terminal within packages/storybook run npm run cypress:open. Cypress app would start and select the markdownEditor.spec.js file and then the test should start.

Flags

  • You might have to run lerna clean && lerna bootstrap && lerna run build as a new devDependency is added.

Screenshots or Video

https://www.loom.com/share/72e05a6e0c014ba6940fb36b757580a8

Related Issues

Author Checklist

  • Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
  • Vital features and changes captured in unit and/or integration tests
  • Commits messages follow AP format
  • Extend the documentation, if necessary
  • Merging to master from fork:branchname
  • Manual accessibility test performed
    • Keyboard-only access, including forms
    • Contrast at least WCAG Level A
    • Appropriate labels, alt text, and instructions

@sanketshevkar
Copy link
Member Author

@irmerk any updates on this?

@jolanglinais
Copy link
Member

@mttrbrts and I are looking into this from #279

@sanketshevkar
Copy link
Member Author

@mttrbrts and I are looking into this from #279

Okay cool. Should I close this pr?

@mttrbrts mttrbrts self-requested a review March 12, 2021 10:56
@mttrbrts
Copy link
Member

@mttrbrts and I are looking into this from #279

Okay cool. Should I close this pr?

No, I've added another commit which completes the basic setup. @rkotangoor @irmerk can you review, please?

@mttrbrts mttrbrts requested review from DianaLease and jolanglinais and removed request for mttrbrts March 12, 2021 13:15
@mttrbrts
Copy link
Member

mttrbrts commented Mar 12, 2021

In theory, Cypress should now report the test results as comments in PRs, and the history will be publicly available at https://dashboard.cypress.io/projects/vrh5u7

@Cronus1007
Copy link
Contributor

Cronus1007 commented Mar 12, 2021

@mttrbrts I have a question that the tests should be controlled via the main directory rather than the storybook package. Whats your opinion regarding this?

@mttrbrts
Copy link
Member

@mttrbrts I have a question that the tests should be controlled via the main directory rather than the storybook package. Whats your opinion regarding this?

That's a good question. For these E2E tests, we want to test the components in context (i.e. in an app), and so storybook is a convenient tool for that. I agree that we should be able to run the tests from a package.json script at the root-level too (I'll add it to this PR).

Unit tests for example are better suited to being situated local to the module / package that they are testing.

@Cronus1007
Copy link
Contributor

Cronus1007 commented Mar 12, 2021

@mttrbrts For unit tests you can have a look at this comment #14 (comment)

@cypress
Copy link

cypress bot commented Mar 12, 2021



Test summary

1 0 0 0


Run details

Project web-components
Status Passed
Commit 095f9f7
Started Mar 12, 2021 1:30 PM
Ended Mar 12, 2021 1:31 PM
Duration 00:24 💡
OS Mac 20.3.0
Browser Electron 87

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@rkotangoor
Copy link
Contributor

@sanketshevkar : could you add instructions to storybook Readme on how to run the cypress tests? Pretty much what you have included in this PR description?

@sanketshevkar
Copy link
Member Author

@sanketshevkar : could you add instructions to storybook Readme on how to run the cypress tests? Pretty much what you have included in this PR description?

Sure would do that 👍.

@sanketshevkar
Copy link
Member Author

@mttrbrts @rkotangoor I've updated the readme file.

@DianaLease
Copy link
Member

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

This is awesome. @mttrbrts what kicks off the test run? Whenever a new PR is opened?

@mttrbrts
Copy link
Member

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

This is awesome. @mttrbrts what kicks off the test run? Whenever a new PR is opened?

Thanks, yes, here's the config from cypress.io
image

@Cronus1007
Copy link
Contributor

@mttrbrts Shall we update this to Travis CI/CD pipeline plus codecov report to the repository whenever a new PR is opened.

Copy link
Member

@jolanglinais jolanglinais left a comment

Choose a reason for hiding this comment

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

Needs investigation by @accordproject/maintainers-ui or @mttrbrts.

@mttrbrts
Copy link
Member

@mttrbrts Shall we update this to Travis CI/CD pipeline plus codecov report to the repository whenever a new PR is opened.

This is a separate issue. Let's focus on getting this change in first.

Copy link
Member

@mttrbrts mttrbrts left a comment

Choose a reason for hiding this comment

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

@sanketshevkar can you fix package-lock.json, please? Otherwise, we're ready to merge this.

@sanketshevkar
Copy link
Member Author

sanketshevkar commented Mar 16, 2021

@sanketshevkar can you fix package-lock.json, please? Otherwise, we're ready to merge this.

I'm not able to find where the merge conflict is exactly located. Can someone help me out?

@Cronus1007
Copy link
Contributor

@sanketshevkar First of all update your branch. This will remove the merge-conflicts .

@sanketshevkar
Copy link
Member Author

@mttrbrts why is it expecting a dco of mine from some other id. I've made all my dco sign off's using [email protected].

@mttrbrts
Copy link
Member

@mttrbrts why is it expecting a dco of mine from some other id. I've made all my dco sign off's using [email protected].

Apologies, I've been trying (and failing!) to fix this properly. I've now manually reviewed and approved the commits.

@sanketshevkar
Copy link
Member Author

@mttrbrts I've updated my branch and created new package-lock.json file. Should I commit that?

@sanketshevkar
Copy link
Member Author

@mttrbrts why is it expecting a dco of mine from some other id. I've made all my dco sign off's using [email protected].

Apologies, I've been trying (and failing!) to fix this properly. I've now manually reviewed and approved the commits.

No, problem. Thanks a lot for helping out.

@jolanglinais
Copy link
Member

So, as of right now, we manually run this locally and it will produce a test result in Cypress? Next steps will be:

  • Set up automatic runs in the Travis (soon to be GitHub Actions) scripts to potentially run in each PR?
  • Continue to iterate on the Cypress test suite?

@mttrbrts
Copy link
Member

So, as of right now, we manually run this locally and it will produce a test result in Cypress? Next steps will be:

  • Set up automatic runs in the Travis (soon to be GitHub Actions) scripts to potentially run in each PR?
  • Continue to iterate on the Cypress test suite?

Yes, that's right. Although the PR integration is already setup (without using Travis or GH Actions). See #282 (comment)

Copy link
Member

@jolanglinais jolanglinais left a comment

Choose a reason for hiding this comment

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

👏🏻

@jolanglinais jolanglinais merged commit 6a6dcc8 into accordproject:master Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants