feat: Update migration for projects with projectId #21193

estrada9166 · 2022-04-26T00:06:18Z

Closes UNIFY-1684

User facing changelog

Additional details

How has the user experience changed?

Do not migrate the user pre-extension if the project has projected on the config, with this change we are not going to display anything related to the pre-extension migration, and also, we are gonna add a custom specPattern in the config file.

The reason behind this change is to keep the spec history on the cloud, simple as possible.

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-04-26T00:06:20Z

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-04-26T00:27:51Z

Test summary

17907 • 0 • 217 • 0 • 2

Run details


Project	cypress
Status	Passed
Commit	`428abdd`
Started	May 3, 2022 10:32 PM
Ended	May 3, 2022 10:47 PM
Duration	14:55 💡
OS	Linux Debian - 10.10
Browser	Multiple

View run in Cypress Dashboard ➡️

Flakiness

	files.cy.js	1
1	... > has implicit existence assertion, retries and throws a specific error when file does not exist for null encoding
	navigation.cy.js	1
1	src/cy/commands/navigation > #visit > window immediately resolves and doesn't reload when visiting the same URL with hashes

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

…ension-if-projectId-exist

…-exist' of github.com:cypress-io/cypress into alejandro/feat/do-not-migrate-preExtension-if-projectId-exist

…jandro/feat/do-not-migrate-preExtension-if-projectId-exist

…ension-if-projectId-exist

lmiller1990

Some questions, code looks pretty thorough.

lmiller1990 · 2022-04-28T05:28:54Z

packages/data-context/src/sources/MigrationDataSource.ts

@@ -62,6 +62,18 @@ export class MigrationDataSource {
    return this.ctx.coreData.migration.legacyConfigForMigration
  }

+  get shouldMigratePreExtension () {
+    return !this.legacyConfig.projectId


I wonder if anyone does nested projectId, like

{ "e2e": { "projectId": "ABC" }, "component": { "projectId": "DEF" } }

Not sure if this is even valid, but if it is, we might need to consider this scenario, too.

According to Brian, this should not be permitted, it needs to be a root level property only

We might need to verify if this actually works in 9.x and see if anyone was using it like that, seems unlikely but you never know.

Eg: I'm saying if it was previously permitted, it's possible someone thought to do this.

I tried the projectId inside the e2e and it works fine in 9.x

The support for this should be something like return !this.legacyConfig.projectId && !this.legacyConfig.e2e?.projectId - how common may this be? I guess it would be safer to support it

That means component.projectId probably works, too. If someone either e2e.projectId OR component.projectId, I think we should not rename, just to be safe.

I have no idea how common this is, but since this is how we make $$$ I don't think we can just assume it's unlikely to happen - we need to get this 100% perfect. I think we should do as you suggested, and also add a check for component.projectId to be 100% safe. Thoughts?

For the moment we are not gonna take into account the CT for the history on the cloud - just the E2E tests; that's why I only update the e2e.projectId

lmiller1990 · 2022-04-28T05:29:55Z

packages/data-context/src/sources/MigrationDataSource.ts

+    return !this.legacyConfig.projectId
+  }
+
+  get migrationOptions () {


Seems like we have this twice, here and also here? https://github.com/cypress-io/cypress/pull/21193/files#diff-c2c1f114b54072e7f036c272e41454586ddba1378ada7bf61d38049e05e3d8f5R68

Also what is the point of this getter? Why not just access shouldMigratePreExtension directly?

lmiller1990 · 2022-04-28T05:33:05Z

packages/data-context/test/unit/sources/migration/format.spec.ts

@@ -19,13 +19,27 @@ describe('formatMigrationFile', () => {
        { text: 'js', highlight: false },
      ])
    })
+
+    it('do not highlight the preExtension', () => {


Suggested change

it('do not highlight the preExtension', () => {

it('do not highlight the preExtension when migratePreExtension is false', () => {

lmiller1990 · 2022-04-28T05:34:47Z

system-tests/projects/migration-e2e-custom-integration-with-projectId/README.md

@@ -0,0 +1,35 @@
+## Migration E2E Custom Integration with projectId
+
+An e2e project with a custom `integrationFolder` named `src` and projectId. It uses the default `testFiles`. We will keep the custom `intergrationFolder`, but it'll be part of `e2e.specPattern`. We will rename their specs to use the `.cy.js` extension.


Don't forget to update this to explain this fixture - it says "We will rename their specs to use the .cy.js extension", which isn't right.

In this case, "this project has as projectId which means we do not migrate to the new .cy.js extension, because this will interfere with the historical data on Cypress Cloud" or something like that.

…ension-if-projectId-exist

lmiller1990

One more comment, but not a blocker.

lmiller1990 · 2022-05-02T01:47:40Z

packages/data-context/src/sources/migration/shouldShowSteps.ts

+    // If the configFile has projectId, we do not want to change the preExtension
+    // so, we can keep the cloud history
+    shouldMigratePreExtension: !config.projectId && !config.e2e?.projectId,
+  }


Should this call this getter? https://github.com/cypress-io/cypress/pull/21193/files#diff-6dae8a6d4d0aac4cb2685a16ea20ceeb091f6113e7e882be5c14f89f579b7e52R69

This file lives on the data-context folder but is not using the dataContext itself - I'll call this a util/helper file

…ension-if-projectId-exist

…jandro/feat/do-not-migrate-preExtension-if-projectId-exist

feat: Update migration for projects with projectId

7ccef03

estrada9166 and others added 11 commits April 25, 2022 20:05

Merge branch '10.0-release' into alejandro/feat/do-not-migrate-preExt…

b21db31

…ension-if-projectId-exist

Merge branch '10.0-release' into alejandro/feat/do-not-migrate-preExt…

e5ae4af

…ension-if-projectId-exist

Add tests

ad420b2

Merge branch 'alejandro/feat/do-not-migrate-preExtension-if-projectId…

7dd01b9

…-exist' of github.com:cypress-io/cypress into alejandro/feat/do-not-migrate-preExtension-if-projectId-exist

Update prettier require, add test for custom specPattern

505f3d1

Merge branch '10.0-release' of github.com:cypress-io/cypress into ale…

0b3df5a

…jandro/feat/do-not-migrate-preExtension-if-projectId-exist

Revert prettier changes

13704c6

Update test

0680611

Merge branch '10.0-release' into alejandro/feat/do-not-migrate-preExt…

d61a359

…ension-if-projectId-exist

Merge branch '10.0-release' into alejandro/feat/do-not-migrate-preExt…

710bd3e

…ension-if-projectId-exist

Add defaultMigrationTransformOptions

5e998bd

estrada9166 marked this pull request as ready for review April 27, 2022 16:34

estrada9166 requested review from a team and tgriesser as code owners April 27, 2022 16:34

estrada9166 requested review from jennifer-shehane, a team and lmiller1990 and removed request for a team and jennifer-shehane April 27, 2022 16:35

Merge branch '10.0-release' into alejandro/feat/do-not-migrate-preExt…

7544114

…ension-if-projectId-exist

lmiller1990 reviewed Apr 28, 2022

View reviewed changes

estrada9166 and others added 2 commits April 28, 2022 10:13

Update with code review

792a46e

Merge branch '10.0-release' into alejandro/feat/do-not-migrate-preExt…

8095c0d

…ension-if-projectId-exist

estrada9166 requested a review from lmiller1990 April 28, 2022 15:42

estrada9166 and others added 4 commits April 28, 2022 11:10

Support nested projectId

94ba04e

Fix test

ac3f650

Merge branch '10.0-release' into alejandro/feat/do-not-migrate-preExt…

e5da9a9

…ension-if-projectId-exist

Merge branch '10.0-release' into alejandro/feat/do-not-migrate-preExt…

531d6c2

…ension-if-projectId-exist

MuazOthman requested a review from flotwig April 29, 2022 15:35

lmiller1990 approved these changes May 2, 2022

View reviewed changes

estrada9166 added 2 commits May 2, 2022 09:22

Merge branch '10.0-release' into alejandro/feat/do-not-migrate-preExt…

94cf553

…ension-if-projectId-exist

Merge branch '10.0-release' into alejandro/feat/do-not-migrate-preExt…

00ae57c

…ension-if-projectId-exist

flotwig approved these changes May 3, 2022

View reviewed changes

Merge branch '10.0-release' of github.com:cypress-io/cypress into ale…

428abdd

…jandro/feat/do-not-migrate-preExtension-if-projectId-exist

estrada9166 merged commit 88ef75c into 10.0-release May 3, 2022

estrada9166 deleted the alejandro/feat/do-not-migrate-preExtension-if-projectId-exist branch May 3, 2022 22:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Update migration for projects with projectId #21193

feat: Update migration for projects with projectId #21193

estrada9166 commented Apr 26, 2022 •

edited

Loading

cypress-bot bot commented Apr 26, 2022

cypress bot commented Apr 26, 2022 •

edited

Loading

lmiller1990 left a comment

lmiller1990 Apr 28, 2022

tgriesser Apr 28, 2022

lmiller1990 Apr 28, 2022

estrada9166 Apr 28, 2022

lmiller1990 Apr 28, 2022

estrada9166 Apr 29, 2022

lmiller1990 Apr 28, 2022

lmiller1990 Apr 28, 2022

lmiller1990 Apr 28, 2022

lmiller1990 Apr 28, 2022

lmiller1990 left a comment

lmiller1990 May 2, 2022

estrada9166 May 2, 2022

	it('do not highlight the preExtension', () => {
	it('do not highlight the preExtension when migratePreExtension is false', () => {

		@@ -0,0 +1,35 @@
		## Migration E2E Custom Integration with projectId

		An e2e project with a custom `integrationFolder` named `src` and projectId. It uses the default `testFiles`. We will keep the custom `intergrationFolder`, but it'll be part of `e2e.specPattern`. We will rename their specs to use the `.cy.js` extension.

feat: Update migration for projects with projectId #21193

feat: Update migration for projects with projectId #21193

Conversation

estrada9166 commented Apr 26, 2022 • edited Loading

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

cypress-bot bot commented Apr 26, 2022

cypress bot commented Apr 26, 2022 • edited Loading

Test summary

Run details

Flakiness

lmiller1990 left a 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

lmiller1990 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estrada9166 commented Apr 26, 2022 •

edited

Loading

cypress bot commented Apr 26, 2022 •

edited

Loading