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

chore: notification telemetry #27026

Merged
merged 16 commits into from Jun 23, 2023

Conversation

lmiller1990
Copy link
Contributor

@lmiller1990 lmiller1990 commented Jun 14, 2023

Additional details

Capture some non-intrusive and non identifying telemetry for App Notifications.

Steps to test

There are two ways a user might navigate to the Debug page:

  • via side navigation
  • via a notification

We want to capture how the user is navigating to the Debug page. That's what you are testing here.

The easiest way to do this is to turn on debugging for cypress:data-context:actions:EventCollectorActions and look for a message that says Recorded machine-linked with the expected event info.

Then you can trigger the event like this:

  1. source: sidebar - just click the Debug icon in from the App. It will launch a mutation that eventually calls the recordEvent action.
  2. source: notification. This is a bit more tricky. You'll need to trigger a notification. You can see how to do that by looking at this PRs test plan. Once you click a notification, it should also record the event with the correct medium.

Example:

image

How has the user experience changed?

PR Tasks

@lmiller1990 lmiller1990 marked this pull request as ready for review June 15, 2023 04:30
@cypress
Copy link

cypress bot commented Jun 15, 2023

28 flaky tests on run #48117 ↗︎

0 27902 1349 0 Flakiness 28

Details:

Fixing ts linting
Project: cypress Commit: 557a8e08e6
Status: Passed Duration: 21:59 💡
Started: Jun 23, 2023 2:10 AM Ended: Jun 23, 2023 2:32 AM
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
Flakiness  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
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  commands/waiting.cy.js • 1 flaky test • 5x-driver-firefox

View Output Video

Test Artifacts
... > errors > throws waiting for the 3rd response Output

The first 5 flaky specs are shown, see all 17 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 June 15, 2023 23:27
@warrensplayer warrensplayer marked this pull request as draft June 16, 2023 15:32
@warrensplayer
Copy link
Contributor

This looks like it will take some sort of Cloud work to implement. Please see internal Slack initiative channel for more details. For now, moving this back to 'draft' state.

@warrensplayer warrensplayer marked this pull request as ready for review June 22, 2023 19:07
@warrensplayer warrensplayer requested a review from a team June 22, 2023 21:45
if (!this.projectBase) {
debug('No projectBase, cannot change url')

return
}

const newUrl = `#/debug`
const params = JSON.stringify({ from: 'notification', runNumber })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Neat idea to pass it through here.

Copy link
Contributor Author

@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.

This looks much cleaner now, thank you @warrensplayer.

It looks like we collect a lot less telemetry than initially - great to see. This looks good to me. I gave it a small test. Let's merge this up and move onto the final QA / acceptance testing.

@lmiller1990 lmiller1990 merged commit ccfdeef into feature/run-notifications-m1 Jun 23, 2023
76 of 78 checks passed
@lmiller1990 lmiller1990 deleted the lmiller/issue-26785 branch June 23, 2023 03:35
@cypress cypress bot mentioned this pull request Jun 23, 2023
3 tasks
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

2 participants