Skip to content
This repository has been archived by the owner on Jul 26, 2022. It is now read-only.

Updates package.json in all packages to run e2e scripts #359

Merged
merged 2 commits into from
Dec 19, 2018

Conversation

vijetmahabaleshwar-okta
Copy link
Contributor

@vijetmahabaleshwar-okta vijetmahabaleshwar-okta commented Dec 17, 2018

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Adding Tests
  • Build related changes
  • CI related changes
  • Documentation changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Removes dependency on certain version of chrome driver to run tests

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Reviewers

@vijetmahabaleshwar-okta vijetmahabaleshwar-okta changed the title Updates package.json in okta-angular Updates package.json in all packages to run e2e script Dec 19, 2018
@vijetmahabaleshwar-okta vijetmahabaleshwar-okta changed the title Updates package.json in all packages to run e2e script Updates package.json in all packages to run e2e scripts Dec 19, 2018
Copy link
Contributor

@jmelberg-okta jmelberg-okta left a comment

Choose a reason for hiding this comment

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

Once the comments are addressed 👉 🚀

"test:e2e": "yarn --cwd test/e2e/harness/ e2e",
"test": "yarn lint && yarn e2e && yarn test:unit",
"pree2e": "yarn build:dependencies",
"e2e": "yarn --cwd test/e2e/harness/ e2e",
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to change this from test:e2e to e2e throughout all of these package updates? By doing so, we're losing out on the naming convention of test:${type} laid out across this monorepo.

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 wanted pree2e script to run automatically since we won't be able to run yarn test:e2e without first running yarn pretest.
The general convention in the yarn world I believe is to have e2e and pree2e scripts.
This is needed for running the e2e tests on bacon.
I could instead do yarn pretest; yarn test:e2e but running yarn e2e alone seemed cleaner.

@@ -33,7 +33,7 @@ module.exports = {
javascriptEnabled: true,
acceptSslCerts: true,
chromeOptions: {
args: ['--disable-gpu --no-sandbox --headless --window-size=1920,1080 --verbose --disable-dev-shm-usage']
args: ['--no-sandbox']
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to bring back the --headless flag here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason, having --headless crashes the chrome on bacon. I have seen a stackoverflow thread talking about the same. Works on travis as well. And locally, it's nice to be able to see the browser in action.

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

Successfully merging this pull request may close these issues.

2 participants