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

feat: support custom workspaceRoot for angular CT #26030

Conversation

barbados-clemens
Copy link
Contributor

@barbados-clemens barbados-clemens commented Mar 6, 2023

Nx issue for ref: nrwl/nx#12113

Additional details

This is the same implementation as #25384 but to solve a different issue, primarly the issue where Nx + Cypress Angular CT is unable to load the workspace root tailwinds config file. This logic is hard coded within angular webpack utils that cypress CT calls.

function getTailwindConfigPath({ projectRoot, root }: WebpackConfigOptions): string | undefined {
  // A configuration file can exist in the project or workspace root
  // The list of valid config files can be found:
  // https://github.com/tailwindlabs/tailwindcss/blob/8845d112fb62d79815b50b3bae80c317450b8b92/src/util/resolveConfigPath.js#L46-L52
  const tailwindConfigFiles = ['tailwind.config.js', 'tailwind.config.cjs'];
  for (const basePath of [projectRoot, root]) {
    for (const configFile of tailwindConfigFiles) {
      // Irrespective of the name project level configuration should always take precedence.
      const fullPath = path.join(basePath, configFile);
      if (fs.existsSync(fullPath)) {
        return fullPath;
      }
    }
  }

  return undefined;
}

The Angular CT handler sets the projectRoot as the workspaceRoot when working with the angular webpack utils which works when cypress is opened in the root of the workspace. But within an nx context Cypress is opened in the project root which means any tailwind config files in the workspace root aren't loaded.

This PR allows for an optional workspaceRoot option to be passed via the cypress config component testing project configuration. if one isn't provided then the default value of projectRoot is used to prevent any other integration/assumptions from breaking but allowing the customizations for tools like Nx to provide the expected behavior when the Cypress isn't opened in the workspaceRoot.

Steps to test

This change does requires changes to the @nrwl/angular nx component testing preset. Mainly the offset value is removed and a custom 'workspaceRoot' is passed to cypress. This can be done within node_modules if really wanted to test it works before this change is in Nx, See: nrwl/nx#15485

make a new nx angular workspace, npx create-nx-workspace@latest --preset=angular
add an tailwind to the workspace root
use a tailwind class in the apps create 'apps//src/app.component.html'
add CT to the app nx g @nrwl/angular:cypress-component-configuration --project=<app-name>
run CT and notice the tailwind styles are not applied, nx component-test <app-name> --watch

cloneable repo: https://github.com/kanidjar/nx-cypress-ct-styling-bug

How has the user experience changed?

No impact for direct consumers, more flexibility for 3rd party tools like Nx

PR Tasks

@barbados-clemens barbados-clemens changed the title fix: support custom workspaceRoot for angular CT feat: support custom workspaceRoot for angular CT Mar 6, 2023
@cypress
Copy link

cypress bot commented Mar 6, 2023

27 flaky tests on run #44554 ↗︎

0 26863 1281 0 Flakiness 27

Details:

fix: support custom workspaceRoot for angular CT
Project: cypress Commit: e96c255db8
Status: Passed Duration: 20:20 💡
Started: Mar 6, 2023 9:28 PM Ended: Mar 6, 2023 9:48 PM
Flakiness  commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-firefox

View Output Video

Test Artifacts
network stubbing > intercepting request > can delay and throttle a StaticResponse Output
Flakiness  cypress/cypress.cy.js • 3 flaky tests • 5x-driver-firefox

View Output Video

Test Artifacts
... > correctly returns currentRetry Output
... > correctly returns currentRetry Output
... > correctly returns currentRetry Output
Flakiness  cypress/cypress.cy.js • 3 flaky tests • 5x-driver-electron

View Output Video

Test Artifacts
... > correctly returns currentRetry Output Video
... > correctly returns currentRetry Output Video
... > correctly returns currentRetry Output Video
Flakiness  create-from-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
... > runs generated spec Output Screenshots Video
Flakiness  specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs Output Screenshots Video

The first 5 flaky specs are shown, see all 14 specs in Cypress Cloud.

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@lmiller1990 lmiller1990 requested a review from a team March 6, 2023 23:57
@jordanpowell88
Copy link
Collaborator

I think this looks good to me @barbados-clemens. The only thing I think we need to add are system-tests using Nx. This will ensure we don't break features like this moving forward and that we can continue to validate it works with Nx.

Copy link
Collaborator

@jordanpowell88 jordanpowell88 left a comment

Choose a reason for hiding this comment

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

LGTM - I want to try to add a few Nx system tests first though before merging

@lmiller1990
Copy link
Contributor

Looks like the nx test is failing. Once this is passing, we can merge it up. Are you interested in picking this one up @jordanpowell88 to help get over the line?

We've also got another issue filed for adding more robust Nx infra to the monorepo.

@mike-plummer
Copy link
Contributor

@jordanpowell88 Are you looking at the nx failure on this one?

@jordanpowell88
Copy link
Collaborator

jordanpowell88 commented Mar 13, 2023

@mike-plummer It's blocked until we do this issue: #26045

Copy link
Contributor

@mike-plummer mike-plummer left a comment

Choose a reason for hiding this comment

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

Looks good, just one stylistic comment

@@ -253,7 +254,7 @@ async function getAngularCliWebpackConfig (devServerConfig: AngularWebpackDevSer

const buildOptions = getAngularBuildOptions(projectConfig.buildOptions, tsConfig)

const context = createFakeContext(projectRoot, projectConfig, logging)
const context = createFakeContext(projectConfig.buildOptions.workspaceRoot || projectRoot, projectConfig, logging)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this just use the buildOptions output from getAngularBuildOptions above (same on line 279)? Slightly cleaner, perhaps

@barbados-clemens barbados-clemens force-pushed the feat/angular-ct-custom-workspace-root branch from 440bfd1 to c7a7c37 Compare March 15, 2023 15:36
@lmiller1990 lmiller1990 merged commit ea8173f into cypress-io:develop Mar 15, 2023
@barbados-clemens barbados-clemens deleted the feat/angular-ct-custom-workspace-root branch March 16, 2023 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants