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

fix(e2e): don't double run cypress tests #28171

Merged
merged 6 commits into from
Feb 23, 2021
Merged

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Nov 19, 2020

This hopefully fixes failing cypress tests running twice in CI due to lovely "cy:run": "(is-ci && cypress run --browser chrome --record) || cypress run --browser chrome" npm scripts we use (--record seems problematic when not used with CYPRESS_PROJECT_ID and CYPRESS_RECORD_KEY env vars

Targets #28169 which should be much more straight forward #28169 was merged, so targets master branch now

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Nov 19, 2020
@pieh pieh added type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Nov 19, 2020
@pieh pieh marked this pull request as ready for review November 19, 2020 10:34
@pieh
Copy link
Contributor Author

pieh commented Nov 19, 2020

I'll just say that I don't love the extra abstraction that I add here (script that replaces cypress run and also need to parse same args and translate them to Cypress Node API), but as far as I know there is no cross platform if-else command that could do something like is-ci ? run-cypress-with-record : run-cypress-without-record

@pieh
Copy link
Contributor Author

pieh commented Nov 19, 2020

Ok, from some chatting - I will drop calling cypress.run from node and instead go with

node scripts/some-good-name-for-the-script.js "cypress run [...] --record" "cypress run [...]" that would check if we are in CI and for existence of needed env vars and run one of those commands based on that (with exaca or so)

@pieh pieh marked this pull request as draft November 19, 2020 11:11
@delete-merged-branch delete-merged-branch bot deleted the branch master November 19, 2020 12:33
@pieh pieh force-pushed the fix/e2e/dont-double-run-in-ci branch from 28ab082 to b51d262 Compare December 20, 2020 10:18
@pieh pieh changed the base branch from chore/record-all-e2e-tests-to-dashboard to master December 20, 2020 10:18
@pieh pieh marked this pull request as ready for review December 20, 2020 10:59
@wardpeet wardpeet merged commit 6a18eab into master Feb 23, 2021
@wardpeet wardpeet deleted the fix/e2e/dont-double-run-in-ci branch February 23, 2021 15:39
pragmaticpat pushed a commit to pragmaticpat/gatsby that referenced this pull request Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants