chore: enable caching for lint task #26791

jordanpowell88 · 2023-05-18T12:49:32Z

Closes Enable caching for lint task #26701

Additional details

This PR enables lint ing with Nx Cloud.

Before this step in CI took:
5m 27s

After this step in CI took:
1m 46s

Steps to test

yarn lint

How has the user experience changed?

PR Tasks

Have tests been added/updated?
Has a PR for user-facing changes been opened in cypress-documentation?
Have API changes been updated in the type definitions?

cypress · 2023-05-18T13:10:29Z

4 flaky tests on run #46907 ↗︎

0	466	6	0	4

Details:

chore: remove no-console rule in root eslintrc
Project: cypress	Commit: `42aefb1fd4`
Status: Passed	Duration: 13:38 💡
Started: Jun 1, 2023 3:44 PM	Ended: Jun 1, 2023 3:58 PM

create-from-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
... > runs generated spec		`Output` `Screenshots` `Video`

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`

cypress-origin-communicator.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
Cypress In Cypress Origin Communicator > cy.origin passivity with app interactions > passes upon test reload mid test execution		`Output` `Screenshots` `Video`

subscriptions/configChange-subscription.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
configChange subscription > on config page > responds to configChange event when viewport is changed		`Output` `Screenshots` `Video`

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

jordanpowell88 · 2023-05-18T15:26:42Z

This PR is a follow up PR after this PR gets merged

jordanpowell88 · 2023-05-18T15:27:30Z

nx.json

+      "runner": "nx-cloud",
+      "options": {
+        "cacheableOperations": [
+          "lint"


this is what enables lint tasks to be cached with Nx Cloud

jordanpowell88 · 2023-05-18T15:28:59Z

scripts/package.json

+{
+  "name": "internal-scripts",
+  "scripts": {
+    "lint": "eslint --ext .js,.ts,.json, ."


The root level package.json lint script also ran this task. This makes the /scripts directory its own lib from an Nx standpoint and moves the lint script here so that it can be cached the same way the other libs do

jordanpowell88 · 2023-05-18T15:29:40Z

package.json

@@ -41,7 +41,7 @@
    "ensure-deps": "./scripts/ensure-dependencies.sh",
    "get-next-version": "node scripts/get-next-version.js",
    "postinstall": "node ./scripts/run-postInstall.js",
-    "lint": "lerna run lint --no-bail --concurrency 2 && eslint --ext .js,.ts,.json, scripts",


the linting of the /scripts folder is now moved inside of /scripts/package.json

this dropped linting of the root-level js/json files which we want to keep

The root level files were not being linted before. We were only running lint across all the packages + manually linting the scripts folder. This just moves the manual linting of the scripts to it's own lint job so the first command will trigger it

emilyrohrbough · 2023-05-18T21:19:32Z

scripts/.eslintrc

@@ -0,0 +1,17 @@
+{


we should be able to re-use the root-level eslint configuration instead of needing to manage another one

Using the root .eslintrc config will throw 340 lint errors:

Unexpected console statement

Synchronous fs calls should not be used in Cypress

We could certainly ignore these other ways but I feel like a separate config for these scripts makes sense given these warnings. Thoughts?

What changed that is is failing with this PR using the same eslint config file and not in develop?

@jordanpowell88 I found this PR here from a few months ago that was adding linting to the scripts directory. In it, a no-console of off was added to an overrides section in the main .eslintrc.js file. Should this now be removed?

@warrensplayer great catch! I removed that rule in 42aefb1

warrensplayer · 2023-06-01T20:16:56Z

package.json

@@ -13,7 +13,7 @@
    "binary-upload": "node ./scripts/binary.js upload",
    "binary-zip": "node ./scripts/binary.js zip",
    "build": "yarn build-npm-modules && lerna run build --stream --no-bail --ignore create-cypress-tests --ignore cypress --ignore \"'@packages/{runner}'\" --ignore \"'@cypress/{angular,react,react18,vue,vue2,mount-utils,svelte}'\" && node ./cli/scripts/post-build.js && lerna run build --stream --scope create-cypress-tests",
-    "build-npm-modules": "lerna run build --scope cypress --scope @cypress/mount-utils && lerna run build --scope \"'@cypress/{angular,react,react18,vue,vue2,svelte}'\"",
+    "build-npm-modules": "lerna run build --scope cypress --scope @cypress/mount-utils --scope @cypress/react && lerna run build --scope \"'@cypress/{angular,react18,vue,vue2,svelte}'\"",


@jordanpowell88 What is this change doing?

Basically because npm/react18 dependsOn npm/react this was failing in CI. It's the same reason why we are building npm/mount-utils before building the rest. By adding Nx into the equation and because we have 2 outputs for the npm libs (their dist and the cli) it was sometimes creating an issue in CI. This change appears to have fixed the inconsistency in CI

How did your changes for the lint task cause this task to start failing? I saw the issue you had in this branch in CI, but could not find it failing in develop. Also, I was looking at the differences between the build step in this branch and develop and saw a few differences. The main one was this:
cypress:build --> @cypress/react:build --> cypress:build
here: https://app.circleci.com/pipelines/github/cypress-io/cypress/53051/workflows/046ba13e-d424-469d-a1af-aeac31bc00ed/jobs/2186964/parallel-runs/0/steps/0-114

Is this a problem?

Yea we have many circular dependencies which is causing this. That is why it requires a bit of change to the build step even though we aren't technically doing anything with build in this PR. This was work that would need to be done either eventually anyways when we cache the build step

warrensplayer · 2023-06-01T20:18:25Z

nx.json

+  "targetDefaults": {
+    "lint": {
+      "inputs": [
+        "default",
+        "{workspaceRoot}/.eslintrc.js"
+      ]
+    },
+    "build": {
+      "dependsOn": [
+        "^build"
+      ]
+    },
+    "build-prod": {
+      "dependsOn": [
+        "^build-prod"
+      ]
+    }
+  },
+  "namedInputs": {
+    "sharedGlobals": [],
+    "default": [
+      "{projectRoot}/**/*",
+      "sharedGlobals"
+    ],
+    "production": [
+      "default",
+      "!{projectRoot}/**/*.spec.ts",
+      "!{projectRoot}/**/*.md",
+      "!{projectRoot}/tsconfig.spec.json",
+      "!{projectRoot}/.eslintrc.json",
+      "!{projectRoot}/.mocharc.{js,json}",
+      "!{projectRoot/cypress.config.{ts,js}",
+      "!{projectRoot/**/*.cy.ts"
+    ]


@jordanpowell88 I am not sure how NX works exactly, so I am not sure how to validate the changes here in nx.json. Can you describe what each part is doing and point to NX documentation that describes how these should be configured. I am guessing it is describing what should or should not be cache for the task, but not sure.

@warrensplayer . Here is the documentation.

But yes in short, yes the namedInputs are used to define sort of "shared" global things that specific tasks would read to determine if a library's cache should be invalidated or not.

For Example: the defaultTargets.lint.inputs array has 2 items that invalidate it's cache. The workspace .eslintrc.js file and default. Default points to the namedInputs.default array which includes any file in that libraries directory ({projectRoot}/**/*) and the sharedGlobals which would be any shared global files that should be included across tasks (in our case it is empty)

Thanks for the explanation. Final question in this section... Is the production option under namedInputs being used by anything?

Not currently. But I went ahead and added what I would expect to be starting values we would use. We can certainly remove this if we want for the time being though

cypress-bot · 2023-06-07T17:25:56Z

Released in 12.14.0.

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

jordanpowell88 mentioned this pull request May 18, 2023

chore: add Nx Cloud #26712

Merged

3 tasks

jordanpowell88 requested a review from a team May 18, 2023 15:25

jordanpowell88 commented May 18, 2023

View reviewed changes

jordanpowell88 force-pushed the jordanpowell88/add-linting branch from b004271 to b1b523d Compare May 18, 2023 16:47

astone123 requested a review from emilyrohrbough May 18, 2023 18:54

emilyrohrbough reviewed May 18, 2023

View reviewed changes

jordanpowell88 force-pushed the jordanpowell88/add-linting branch from d33a71f to a4d5817 Compare May 22, 2023 19:17

jordanpowell88 marked this pull request as draft May 23, 2023 12:36

jordanpowell88 marked this pull request as ready for review May 23, 2023 14:33

astone123 approved these changes May 26, 2023

View reviewed changes

jordanpowell88 added 12 commits May 31, 2023 15:30

chore: add empty nx.json [run ci]

872833e

chore: add lint caching [run ci]

03e67a0

chore: add target defaults and namedInputs

c18ff43

chore: remove sharedGlobals

a0152b7

chore: add devDeps to cli

3b18ca4

chore: update build-npm-modules script

7bce65e

chore: update build-npm-modules script

0c6b748

chore: add build global dependsOn

bf670a3

chore: add ts-ignore comment

2f86a75

chore: update build-prod targetDefaults

764e961

chore: update build-npm-modules script

542a38d

chore: update build-npm-modules script

a66a89c

jordanpowell88 force-pushed the jordanpowell88/add-linting branch from 0b744ad to a66a89c Compare May 31, 2023 19:34

chore: remove no-console rule in root eslintrc

42aefb1

jordanpowell88 requested review from emilyrohrbough and warrensplayer June 1, 2023 15:03

warrensplayer reviewed Jun 1, 2023

View reviewed changes

warrensplayer approved these changes Jun 2, 2023

View reviewed changes

jordanpowell88 merged commit fd63f19 into develop Jun 2, 2023

jordanpowell88 deleted the jordanpowell88/add-linting branch June 2, 2023 20:47

cypress-bot bot locked as resolved and limited conversation to collaborators Jun 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: enable caching for lint task #26791

chore: enable caching for lint task #26791

jordanpowell88 commented May 18, 2023

cypress bot commented May 18, 2023 •

edited

Loading

jordanpowell88 commented May 18, 2023

jordanpowell88 May 18, 2023

jordanpowell88 May 18, 2023

jordanpowell88 May 18, 2023

emilyrohrbough May 18, 2023

jordanpowell88 May 23, 2023

emilyrohrbough May 18, 2023

jordanpowell88 May 22, 2023

emilyrohrbough May 22, 2023

warrensplayer Jun 1, 2023

jordanpowell88 Jun 1, 2023

warrensplayer Jun 1, 2023

jordanpowell88 Jun 1, 2023

warrensplayer Jun 2, 2023

jordanpowell88 Jun 2, 2023

warrensplayer Jun 1, 2023

jordanpowell88 Jun 1, 2023

warrensplayer Jun 2, 2023

jordanpowell88 Jun 2, 2023

cypress-bot bot commented Jun 7, 2023

chore: enable caching for lint task #26791

chore: enable caching for lint task #26791

Conversation

jordanpowell88 commented May 18, 2023

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress bot commented May 18, 2023 • edited Loading

4 flaky tests on run #46907 ↗︎

jordanpowell88 commented May 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress-bot bot commented Jun 7, 2023

cypress bot commented May 18, 2023 •

edited

Loading