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

chore(WEBRTC-366): testing workflow #143

Merged
merged 15 commits into from
Feb 12, 2021
Merged

chore(WEBRTC-366): testing workflow #143

merged 15 commits into from
Feb 12, 2021

Conversation

DeividVeloso
Copy link
Contributor

@DeividVeloso DeividVeloso commented Jan 26, 2021

  • Add cypress on webrtc-test-release.yml to run e2e tests

πŸ“ To Do

  • All linters pass
  • All tests pass
  • Change documentation based on my changes

βœ‹ Manual testing

  1. Go to Actions tab and choose the webrtc-test-release.yml
  2. Click em run workflow and see if you get green.

🦊 Browser testing

Desktop

  • Edge (latest)
  • Chrome
  • Firefox
  • Safari

@SuaYoo SuaYoo self-requested a review January 27, 2021 16:16
Copy link
Contributor

@SuaYoo SuaYoo left a comment

Choose a reason for hiding this comment

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

It looks like the changes from https://github.com/team-telnyx/webrtc/pull/142/files made it into this PR as well, can you remove those commits or change the base branch?

browser: edge
headless: true
env: STORYBOOK_USERNAME=${{ secrets.NPM_CI_STORYBOOK_USERNAME }},STORYBOOK_PASSWORD=${{ secrets.NPM_CI_STORYBOOK_PASSWORD }},STORYBOOK_DESTINATION=${{ secrets.NPM_CI_STORYBOOK_DESTINATION }}
build:
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like it could be consolidated with unit-tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer to keep separated, I did 3 steps:

  • unit-test
  • e2e
  • build/release

@DeividVeloso DeividVeloso requested a review from SuaYoo January 28, 2021 21:26
.env
yarn.lock
Copy link
Contributor

Choose a reason for hiding this comment

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

Why add the lockfile to gitignore? Also, it looks like the file was committed to this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we use the command npm ci we need to remove the yarn.lock, because the GITHUB/ACTIONS complain about that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Run cypress-io/github-action@v2
/usr/bin/yarn --frozen-lockfile
yarn install v1.22.5
warning package-lock.json found. Your project contains lock files generated by tools other than Yarn. It is advised not to mix package managers in order to avoid resolution inconsistencies caused by unsynchronized lock files. To clear this warning, remove package-lock.json.
[1/4] Resolving packages...
warning cypress > @cypress/request > [email protected]: this library is no longer supported
error Your lockfile needs to be updated, but yarn was run with `--frozen-lockfile`.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
Error: The process '/usr/bin/yarn' failed with exit code 1
`
``

Copy link
Contributor

Choose a reason for hiding this comment

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

I think they meant you should remove package-lock.json file instead, as it could potentially be used instead of yarn.lock.

@DeividVeloso DeividVeloso requested a review from SuaYoo January 29, 2021 17:44
@DeividVeloso DeividVeloso merged commit 43ceea7 into main Feb 12, 2021
@DeividVeloso DeividVeloso deleted the WEBRTC-336 branch February 12, 2021 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants