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: avoid ANSI character output when running the binary smoke test f… #28994

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

AtofStryker
Copy link
Contributor

@AtofStryker AtofStryker commented Feb 22, 2024

…or eecution environments that set FORCE_COLOR [run ci]

Additional details

Fixes an issue with env pollution inside the cypress verify step of the binary. After the electron update, something under the hood changed with how environment variables are hydrated into the process where not all options before were being respected inside the process. In particular, this causes issues with the FORCE_COLOR option that can live inside the node process. Previously, FORCE_COLOR on the previous version of electron was not respected inside the execa process, but now it is. This causes the ping number to be colored yellow via our error templating, causing an unexpected mismatch on the lines contained in the verify check. This is only an issue if FORCE_COLOR is set inside the process, which happens to be the case for nx users. Here is a sample diff of the process.env differences between invoking cypress via the nx plugin vs invoking cypress directly.

// nx process.env (from cypress-test-tiny)
{
  env: {
    NX_CLI_SET: 'true',
    NX_LOAD_DOT_ENV_FILES: 'true',
    NX_TASK_TARGET_TARGET: 'e2e',
    npm_config_inspect_brk: 'true',
    NX_TASK_HASH: '18267722001484630910',
    NX_TASK_TARGET_PROJECT: 'cypress-test-tiny',
    NX_WORKSPACE_ROOT: '/Users/$NAME/cypress-test-tiny',
    npm_command: 'run-script',
    FORCE_COLOR: 'true',
    npm_lifecycle_script: 'nx e2e',
    LERNA_PACKAGE_NAME: 'cypress-test-tiny',
  }
}
// cypress process.env (from cypress-test-tiny)
{
  env: {
    npm_command: 'exec',
    npm_lifecycle_script: 'cypress',
  }
}

Since the verify job does not need knowledge of ANSI coloring and we mainly want to make sure the verify passes, we are opting to set FORCE_COLOR to 0 in order for the verify messaging to pass and make the logic less dependent on user environments

Steps to test

Likely the best way to test is setting up cypress-test-tiny and installing the prebuild binary mentioned in this commit. once installed, run yarn nx init and install @nx/cypress. Then install via yarn and execute yarn cypress:run. The binary verification should pass.

I have also added a unit test to verify FORCE_COLOR is off. We could add a system test that leverages a binary test against an nx project, but the time to set that up locally is a bit tedious at the moment and think we get the coverage we need from this plus some of our internal repos.

How has the user experience changed?

PR Tasks

…or eecution environments that set FORCE_COLOR [run ci]
@AtofStryker
Copy link
Contributor Author

AtofStryker commented Feb 22, 2024

the other OS platforms are marked failing because I cancelled them to conserve resources as we know they built fine from previous runs

@jennifer-shehane jennifer-shehane merged commit c1304b2 into develop Feb 22, 2024
96 of 108 checks passed
@jennifer-shehane jennifer-shehane deleted the fix/force_colors_on_verify branch February 22, 2024 19:29
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Feb 22, 2024

Released in 13.6.6.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v13.6.6, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Feb 22, 2024
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.

"Cypress verification failed" after upgrading to 13.6.5 with use of NX
3 participants