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: run cypress headless in ci #30426

Closed
wants to merge 3 commits into from
Closed

Conversation

eschutho
Copy link
Member

@eschutho eschutho commented Sep 28, 2024

SUMMARY

Fixes CI for cypress by running it in headless mode.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@eschutho eschutho marked this pull request as ready for review September 28, 2024 19:50
@eschutho eschutho closed this Sep 29, 2024
@eschutho eschutho reopened this Sep 29, 2024
@@ -61,7 +63,7 @@ def run_cypress_for_test_file(
os.environ.pop("CYPRESS_RECORD_KEY", None)
cmd = (
f"{XVFB_PRE_CMD} "
f"{cypress_cmd} --browser {browser} "
f"{cypress_cmd} --browser {browser} --headless "
Copy link
Member

Choose a reason for hiding this comment

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

@eschutho @geido Would changing this setting to --headless break the screenshots used by Applitools and Cypress dashboard?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was testing to see if that was the case, but so far haven't seen any test failures related to it. I think it should be able to take screenshots headlessly, is my guess. This is actually the default flag, so this may already be running headlessly.

@@ -44,7 +44,9 @@ def run_cypress_for_test_file(
os.environ["TERM"] = "xterm"
os.environ["ELECTRON_DISABLE_GPU"] = "true"
build_id = generate_build_id()
browser = os.getenv("CYPRESS_BROWSER", "chrome")
browser = os.getenv("CYPRESS_BROWSER", "electron")
Copy link
Member

Choose a reason for hiding this comment

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

a bit concerned to bring yet another engine in the mix, specifically for tests/automation. Supporting two is already a challenge, and thinking now that we should just pick one engine, either Chrome or Firefox, and use is consistently for thumbnail-generation, alert & reports and in e2e tests

Copy link
Member

Choose a reason for hiding this comment

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

Also confused about what electron --headless means, isn't electron a "head" by definition? Did a tiny bit of research just now, and Electron uses Blink (a fork of webkit), Chrome/Chromium use Blink as well, and FF uses Gecko. So chrome/chromium/electron in headless more should be pretty much the same AFAIK

@eschutho
Copy link
Member Author

eschutho commented Oct 7, 2024

Closing this as per the concerns about having a third engine in the mix.

@eschutho eschutho closed this Oct 7, 2024
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.

4 participants