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: Component Testing banner #26625

Merged
merged 34 commits into from
May 5, 2023
Merged

feat: Component Testing banner #26625

merged 34 commits into from
May 5, 2023

Conversation

mike-plummer
Copy link
Contributor

@mike-plummer mike-plummer commented Apr 28, 2023

Additional details

Note: The majority of files touched in this PR are just renaming the loginConnectStore to userProjectStateStore

Creating a new banner to help inform e2e users about component testing. This new banner will display under the following conditions:

  1. No higher-priority banners to display (connection warnings, existing ACI banners, etc)
  2. Cypress use > 4 days
  3. At least 2 days since a previous banner of another type (if any) has been dismissed
  4. At least 1 day since the Nav/CI popup has displayed (if it was previously shown)
  5. Currently in e2e mode
  6. CT is not configured (no component key in config)
  7. At least one supported CT framework exists (anything auto-detected by the onboarding wizard)

Steps to test

  1. Create a new project, name it distinctively (eg ct-banner). Add dependencies that would satisfy at least one of our auto-detected frameworks - for example, next or react + webpack.
  2. Open project in Cypress into e2e mode, scaffold example specs, and close Cypress.
  3. Open Cypress app data location, find project's state.json file (on Mac this will live at ~/Library/Application\ Support/Cypress/cy/production/projects/{project}/state.json, or use Developer Tools > View App Data menu item from launchpad). It should look like this:
{
  "firstOpened": 1682966839869,
  "lastOpened": 1682966839869,
  "lastProjectId": null,
  "specFilter": ""
}
  1. Modify the firstOpened timestamp to a value >= 4 days ago (easy way is to delete the first digit) and save
  2. Reopen project into e2e mode - an ACI banner should display (which one depends on whether you're logged in - doesn't matter for this procedure).
    • Refresh the page, verify that banner still displays.
    • Close the banner using the "X".
    • Refresh and verify no banner displays.
    • Close Cypress
  3. state.json file should now look like this - adjust the dismissed timestamp to a value >= 2 days ago
{
  "firstOpened": 682966839869,
  "lastOpened": 1682966881904,
  "lastProjectId": null,
  "specFilter": "",
  "banners": {
    "aci_082022_connectProject": {
      "lastShown": 1682966943916,
      "dismissed": 682967079384
    }
  }
}
  1. Reopen project into e2e mode, new CT Available banner should display
    • Banner should detect correct framework, use correct name & logo
    • state.json should now look like this:
	{
  "firstOpened": 682966839869,
  "lastOpened": 1682967155424,
  "lastProjectId": null,
  "specFilter": "",
  "banners": {
    "aci_082022_connectProject": {
      "lastShown": 1682966943916,
      "dismissed": 682967079384
    },
    "ct_052023_available": {
      "lastShown": 1682967157281
    }
  }
}
  1. Click "Quick Setup" - verify you are sent into the CT setup flow in launchpad. Go back to e2e mode
  2. Click "Read the docs" - verify CT docs page is opened
  3. Click "Not interested..." link
    • Verify survey is opened.
    • Verify that banner is now hidden in Cypress. Refresh page, verify banner is still not shown.
  4. Close Cypress, edit state.json to remove dismissed key from ct_052023_available banner. Reopen project
  5. Banner should once again show. Click "X" to dismiss, verify banner is hidden and does not reappear on page refresh.
  6. Extra credit - by deleting the dismissed key and reopening you can validate the various conditions outlined above for when the banner should display/not display (CT not already configured, matching framework detected, etc)

How has the user experience changed?

Screenshot 2023-05-03 at 3 45 41 PM

PR Tasks

@cypress
Copy link

cypress bot commented Apr 28, 2023

4 flaky tests on run #46056 ↗︎

0 5193 77 0 Flakiness 4

Details:

Fix button size
Project: cypress Commit: c4db763a28
Status: Passed Duration: 13:22 💡
Started: May 5, 2023 8:14 PM Ended: May 5, 2023 8:27 PM
Flakiness  commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-electron

View Output Video

Test Artifacts
network stubbing > intercepting request > can delay and throttle a StaticResponse Output Video
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

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

mike-plummer and others added 10 commits May 1, 2023 08:40
* Hook up switch to CT mutation
* Persist dismissal on survey click
* add link placeholders
* Support new endpoint for identifiable events
* Collect event when button used in CT banner
* Refactor machine id into coreData so it can be queried using gql
@lmiller1990 lmiller1990 changed the title feat: Component Testing banner feat: [No Merge, pending design updates] Component Testing banner May 2, 2023
@mike-plummer mike-plummer requested a review from a team May 2, 2023 20:20
@mike-plummer mike-plummer marked this pull request as ready for review May 2, 2023 20:20
@@ -20,8 +20,9 @@
},
"dependencies": {},
"devDependencies": {
"@cypress-design/vue-icon": "0.20.0",
"@cypress-design/vue-statusicon": "0.3.0",
"@cypress-design/vue-button": "0.9.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

Copy link
Contributor

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

Minor comments, I will test it now.

onMounted(() => {
updateBannerState('lastShown')
onMounted(async () => {
await updateBannerState('lastShown')
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this await actually does anything - the hooks are async, onMounted will run the execution will go on without waiting. Was this just a style change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly stylistic - I made the updateBannerSync function async, so flowed the await up through all usages just so the Promise-nature isn't lost. Probably a no-op for now, but if additional logic is added to onMounted in the future it will at least execute in order within the callback

@@ -100,6 +100,14 @@ export class WizardActions {

this.resetWizard()

await this.initializeFramework()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this somehow related to this feature, or is it just a drive by bug fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pulled out the new initializeFramework action so we could trigger the existing "framework & bundler detection" logic while in e2e mode without touching other wizard stuff. I probably could have just used the existing initialize action since all it does is a reset then this logic, but I wanted to have a clean action in case there were updates to the wizard initialization flow in the future that we wouldn't want to fire while in e2e

packages/data-context/src/data/coreDataShape.ts Outdated Show resolved Hide resolved
@lmiller1990
Copy link
Contributor

It worked

image

I found it was not working - I forgot to do yarn install in my new project. The project must not just have the deps in package.json but also have installed them.

I did the full workflow and got this, which is expected, since I don't have a CLI installed in the project

image

Looks like it works to me! Curious to track how many people will see this banner.

Copy link
Contributor

@warrensplayer warrensplayer left a comment

Choose a reason for hiding this comment

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

I am still working on actually setting up a project of my own to test, but wanted to share these comments.

The big question I have is about our own monorepo. When I open the App or Launchpad packages in E2E mode then I get a banner to set up Angular. (see screenshot). Is it detecting the Angular in our dependencies from our CT mount adaptor? Should the logic be more specific on what it is searching for monorepos? Should we test this via some system tests to make sure it is giving accurate information with different types of project setups?

image

packages/app/src/specs/SpecsListBanners.vue Outdated Show resolved Hide resolved
Comment on lines +24 to +25
"@cypress-design/vue-icon": "0.22.1",
"@cypress-design/vue-statusicon": "0.4.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why were vue-icon and vue-statusicon updated with this PR? Are these versions that we should have upgraded with the move to Tailwind?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bumped those as part of troubleshooting the issues with @cypress-design/vue-button, and now that we're pulling in the latest @cypress-design/css I figured we'd want to keep the updates in case there were cross-package compatibility issues

packages/app/src/specs/SpecsListBanners.cy.tsx Outdated Show resolved Hide resolved
* Encapsulate status determination inside `PromptManager`
* Use existing `isCTConfigured` flag to standardize detection logic
* Fix incorrect icon rotation in Alert header
@mike-plummer mike-plummer changed the title feat: [No Merge, pending design updates] Component Testing banner feat: Component Testing banner May 4, 2023
@mike-plummer
Copy link
Contributor Author

@warrensplayer Made a small update to use an existing isCTConfigured flag rather than deriving my own - after that change I can't reproduce the banner showing up when CT is already set up like you described. LMK if you still see it

@warrensplayer
Copy link
Contributor

@warrensplayer Made a small update to use an existing isCTConfigured flag rather than deriving my own - after that change I can't reproduce the banner showing up when CT is already set up like you described. LMK if you still see it

Looks like that fixed it for me as well.

@lmiller1990 lmiller1990 self-requested a review May 4, 2023 21:51
Copy link
Contributor

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

Good catch @warrensplayer - we definitely don't want to recommending configuring CT if it is already configured.

The rest of this looks good to me!

Copy link
Contributor

@warrensplayer warrensplayer left a comment

Choose a reason for hiding this comment

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

All of my concerns were addressed

Copy link
Contributor

@marktnoonan marktnoonan left a comment

Choose a reason for hiding this comment

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

Whoops I just noticed a comment I had in draft. I haven't gone through the code in detail but based on the other reviews I don't see a need to, so just a comment about the button size.

@mike-plummer mike-plummer merged commit adc4c5e into develop May 5, 2023
76 of 78 checks passed
@mike-plummer mike-plummer deleted the mikep/26511-ct-banner branch May 5, 2023 21:07
@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 12, 2023

Released in 12.12.0.

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

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators May 12, 2023
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.

Component Testing banner
5 participants