chore(launchpad): work on infra for scaffold tests #20818

lmiller1990 · 2022-03-29T07:27:21Z

Closes https://cypress-io.atlassian.net/browse/UNIFY-1305

User facing changelog

Add some code to assert the correct files are created for new projects. It makes it quickly and easy to add new projects and assert the scaffolded code is what we expect.

Additional details

See this file for more details on my idea: https://github.com/cypress-io/cypress/pull/20818/files#diff-4b0c978082d6a9bf6dd7dfb2211caa0bf93f93a20da215a9907bd3e3c840137f

Basically, I added a function in TestActions that is kind of like snapshots for the scaffolded files. If a project hasn't got any expected files (in the cypress-expected directory) we just copy the scaffolded ones there, like a snapshot, for next time the test runs.

If the files do exist, we assert the scaffolded files are the same as the expected ones.

When successful, a test looks like this:
s

A failure shows the diff:

How has the user experience changed?

PR Tasks

Have tests been added/updated?
Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
Has a PR for user-facing changes been opened in cypress-documentation?
Have API changes been updated in the type definitions?
Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2022-03-29T07:27:23Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

cypress · 2022-03-29T08:06:24Z

Test summary

17792 • 0 • 217 • 0 • 2

Run details


Project	cypress
Status	Passed
Commit	`044bd4d`
Started	Apr 1, 2022 3:22 PM
Ended	Apr 1, 2022 3:34 PM
Duration	12:03 💡
OS	Linux Debian - 10.10
Browser	Multiple

View run in Cypress Dashboard ➡️

Flakiness

	xhr.cy.js	1
1	... > no status when request isnt forced 404
	files.cy.js	1
1	... > has implicit existence assertion, retries and throws a specific error when file does not exist for null encoding

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

…NIFY-1305

lmiller1990 · 2022-03-30T03:49:05Z

packages/launchpad/cypress/e2e/scaffold-project.cy.ts

+  })
+}
+
+describe('scaffolding new projects', () => {


We need more to express the scaffolded CT cypress.config.js, but since that's changing soon with the CT Arch project, I'll add those when we actually implement the changes.

flotwig

very cool!

flotwig · 2022-03-30T14:11:26Z

packages/data-context/src/actions/TestActions.ts

+// Also defined in lib/fixtures, but we do not want to
+// import from system-tests into code we ship in production.
+const cyTmpDir = path.join(tempDir, 'cy-projects')


why are we shipping test code in production? this seems very wrong

Yeah not super ideal - I will move this into a cy.task.

Ok @flotwig, I put it in a cy.task: 3810fd6

flotwig · 2022-03-30T14:16:17Z

packages/data-context/src/actions/TestActions.ts

+    const files = (
+      await Promise.all([
+        globby(path.join(this.ctx.currentProject, 'cypress'), { onlyFiles: true }),
+        globby(path.join(this.ctx.currentProject, 'cypress.config.*'), { onlyFiles: true }),


globby only works with forward slashes, so this will break for Windows contributors: https://github.com/sindresorhus/globby#api

suggested to do something like https://stackoverflow.com/a/63251716/3474615 to ensure these are POSIX paths

Done. Please see https://github.com/cypress-io/cypress/pull/20818/files#diff-205e537e85adf50907257ece5985a0576754cefa95b0f691cc68bb8c11b7c3d0R98-R118

Shouldn't we just use path.posix.join all the time? Any case to not use it?

I think we should use the real, native path wherever possible, since it's what most APIs expect, and also what users expect. So backslashes (path.win32.sep) on Windows, forward slashes (path.posix.sep) on Linux/macOS. The only exceptions to this that I know of are globby and when we're codegenning paths for import/require.

Sounds good.

packages/launchpad/cypress/e2e/scaffold-project.cy.ts

flotwig · 2022-03-30T14:20:30Z

packages/data-context/package.json

    "dataloader": "^2.0.0",
    "dayjs": "^1.9.3",
    "dedent": "^0.7.0",
+    "disparity": "^3.0.0",


this and temp-dir should be in devDependencies since this is test code

I moved these from data-context to launchpad when moving the test code to a task, and also moved them into devDependencies, so they are no longer in dependencies.

BlueWinds · 2022-03-30T15:10:18Z

packages/data-context/src/actions/TestActions.ts

+export class TestActions {
+  constructor (private ctx: DataContext) {}
+
+  async #snapshotScaffoldedFiles (expectedScaffoldDir: string, filesToDiff: FileToDiff[]): Promise<SnapshotScaffoldTestResult> {


Why are these functions named with # at the start? I haven't seen this before, and a cursory search suggests it may come from ruby or java?

If it's to denote these functions as internal, then I'd rather use an _, which I've seen in JS before, or even better, don't define them on the class at all. Could put them up above, since they don't use this at all.

It's a new shortcut for doing private methodName() {... I had to look it up too. Honestly I prefer private just because it's more visually distinct, but absent of a coding styleguide to back me up, I was gonna keep it to myself 😄

I think JS lends itself poorly to OOP / classes, and the less of it we do the better our code is going to come out. :P

Sure, we can just use private. My reasoning here was it makes more sense to use a native JS feature, which has runtime privacy guarantees, instead of a compile time only, proprietary TS feature where possible, but I don't think it makes too big a difference either way.

I ended up moving everything into a task: https://github.com/cypress-io/cypress/pull/20818/files#diff-205e537e85adf50907257ece5985a0576754cefa95b0f691cc68bb8c11b7c3d0 which is just a function, no class, so this problem is gone.

I do think it's better to use native JS runtime features where we can over TS compile time features, though, but we can discuss that at some point; this PR no longer uses either.

BlueWinds · 2022-03-30T15:58:37Z

system-tests/projects/pristine/expected-cypress-ts-component/cypress.config.ts

@@ -0,0 +1,11 @@
+import { defineConfig } from 'cypress'


So this is unrelated to the PR, but I think it would be a good idea to have a comment about what defineConfig actually does and why people should use it. As far as I understand it, its only job is to provide typing information, and is recommended but not required?

We have a lot of comments in the other generated files, and it feels strange that the most important one doesn't explain what's going on in here at all.

Yep, it's just for type interface - it's a no-op.

It might be good to explain what this does with a comment in the generated code, but changing the actual content of the scaffolded files needs to go through DX and friends. Luckily, if we do, it's trivial to update these tests with the infrastructure introduced here.

Let's follow up with DX internally.

…NIFY-1305

lmiller1990 · 2022-03-30T23:28:33Z

@BlueWinds @flotwig I addressed all the feedback, summary:

moved test code to a task - so we do not ship test code anymore
moved devDependencies to correct location in package.json
using path.posix.join instead of path.join

Can you please re-review? Thanks!

Edit: CI is having some problems (we discussed internally). Let's wait until it's green, then 👀 and ✅.

lmiller1990 · 2022-03-31T05:27:54Z

Ok, it's green again! 🙏

ZachJW34

Looks great! Could this system be used for some of the existing system-test projects we have like vue-cli-{configured,unconfigured}? Had a few minor comments but going to approve.

packages/graphql/tsconfig.json

packages/launchpad/cypress/e2e/scaffold-project.cy.ts

flotwig · 2022-03-31T21:41:15Z

packages/launchpad/cypress/tasks/snapshotsScaffold.ts

 import { cyTmpDir } from '@tooling/system-tests/lib/fixtures'

-const systemTestsDir = path.join(__dirname, '..', '..', '..', '..', 'system-tests', 'projects')
+const systemTestsDir = path.posix.join(__dirname, '..', '..', '..', '..', 'system-tests', 'projects')


on Windows, __dirname will have backslashes in it, and path.posix.join will not do what is expected - it will treat the entire __dirname as a single filename:

I'd just keep this as a native path.join and convert to posix paths at only the places necessary.

Right, I see - thanks.

Ok, I think I've done this right now. We only use posix for the globby globs now. Can you please double check? Thanks.

lmiller1990 · 2022-04-03T20:33:36Z

One more set of 👀 @flotwig 🙏

lmiller1990 · 2022-04-05T10:49:54Z

Still managed to mess up the path stuff and broke windows. Fixing... #20914

* 10.0-release: (92 commits) chore: remove dependency-tree dep (#20905) chore(launchpad): work on infra for scaffold tests (#20818) fix: build mjs in the cli (#20889) fix(unify): Cypress version link should point to Cypress doc's changelog (#20869) fix: windows build (#20854) fix: add index.mjs to the published files of cli (#20884) refactor: lift indexHtmlFile up to component, add validation (#20870) fix: allow migration of pluginsFile using `env` properties (#20770) fix: viewport from CLI on CT (#20849) Don't communicate if process isn't connected Remove config.get Update packages/data-context/src/data/ProjectLifecycleManager.ts fix: git data source unit test failure (#20875) add comment for autoBindDebug fix: Ensuring current browser is synchronized between app and launchpad (#20830) Fix missed await on merge conflict resolution test(unification): move record keys to contexts (#20860) test: move record keys to contexts (#20859) PR comments Fix tests that visit app without a configured testing type ...

chore: work on infra for scaffold tests

3035b1e

lmiller1990 added 3 commits March 29, 2022 17:28

revert

5046ce0

wip

7420dc8

ignore scaffolded files in linting

5d05d81

lmiller1990 added 9 commits March 30, 2022 10:27

Merge remote-tracking branch 'origin/10.0-release' into lmiller1990/U…

2a6360e

…NIFY-1305

add snapshot for plain TS project

94878d8

add more tests

f3b0a73

linting

e7702b5

add temp-dir dep to data-context

605558f

exclude scaffolded assets from linting

8d08812

bump server tsconfig lib to es2019

b8c5453

bump graphql tsconfig lib

957ab51

add missing dependency

699c6dd

lmiller1990 requested review from BlueWinds, flotwig and marktnoonan March 30, 2022 03:47

lmiller1990 marked this pull request as ready for review March 30, 2022 03:47

lmiller1990 requested review from a team as code owners March 30, 2022 03:47

lmiller1990 commented Mar 30, 2022

View reviewed changes

flotwig suggested changes Mar 30, 2022

View reviewed changes

BlueWinds suggested changes Mar 30, 2022

View reviewed changes

lmiller1990 added 6 commits March 31, 2022 08:12

Merge remote-tracking branch 'origin/10.0-release' into lmiller1990/U…

d2b8708

…NIFY-1305

use task to avoid test code leaking into data-context prod

3810fd6

remove unused code

0c9a243

remove deps

5f16689

make currentProject cross platform with path.posix and path.sep

5b3a06b

posix.join

8e64cf3

use path.posix everywhere

ae5931c

lmiller1990 requested review from BlueWinds and flotwig March 30, 2022 23:29

lmiller1990 added 2 commits March 31, 2022 09:48

update types

3c5de3b

Merge branch '10.0-release' into lmiller1990/UNIFY-1305

c32483a

ZachJW34 previously approved these changes Mar 31, 2022

View reviewed changes

packages/graphql/tsconfig.json Show resolved Hide resolved

packages/launchpad/cypress/e2e/scaffold-project.cy.ts Outdated Show resolved Hide resolved

flotwig suggested changes Mar 31, 2022

View reviewed changes

correct paths and add ct framework to expected dir

1af1e6c

lmiller1990 dismissed ZachJW34’s stale review via 1af1e6c April 1, 2022 01:08

ZachJW34 and others added 2 commits April 1, 2022 10:04

Merge branch '10.0-release' into lmiller1990/UNIFY-1305

283f534

fix scaffolded files from indexHtmlFile merge

044bd4d

ZachJW34 approved these changes Apr 1, 2022

View reviewed changes

BlueWinds approved these changes Apr 4, 2022

View reviewed changes

lmiller1990 merged commit 1645431 into 10.0-release Apr 4, 2022

lmiller1990 deleted the lmiller1990/UNIFY-1305 branch April 4, 2022 23:42

lmiller1990 mentioned this pull request Apr 5, 2022

chore(app): fixing windows CI regression on 10.0-release #20914

Merged

chore(launchpad): work on infra for scaffold tests #20818

chore(launchpad): work on infra for scaffold tests #20818

Uh oh!

Conversation

lmiller1990 commented Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

Uh oh!

cypress-bot bot commented Mar 29, 2022

Uh oh!

cypress bot commented Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test summary

Run details

Flakiness

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flotwig left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flotwig Mar 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BlueWinds Mar 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lmiller1990 Mar 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lmiller1990 Mar 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lmiller1990 Mar 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lmiller1990 commented Mar 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lmiller1990 commented Mar 31, 2022

Uh oh!

ZachJW34 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

lmiller1990 commented Mar 29, 2022 •

edited

Loading

cypress bot commented Mar 29, 2022 •

edited

Loading

flotwig Mar 31, 2022 •

edited

Loading

BlueWinds Mar 30, 2022 •

edited

Loading

lmiller1990 Mar 30, 2022 •

edited

Loading

lmiller1990 Mar 30, 2022 •

edited

Loading

lmiller1990 Mar 30, 2022 •

edited

Loading

lmiller1990 commented Mar 30, 2022 •

edited

Loading