Skip to content

fix: meaningful recordings integration tests - #885

Merged
pauldambra merged 9 commits into
masterfrom
fix/meaningful-recordings-integration-tests
Nov 13, 2023
Merged

pauldambra merged 9 commits into
masterfrom
fix/meaningful-recordings-integration-tests

Conversation

@pauldambra

Copy link
Copy Markdown
Member

I want to get to a place where the session id bug fixed in #878 would be impossible to merge.

Our session recording integration tests currently check that

  • when you type in a box
  • PostHog sends two things to capture

They don't even have to be $snapshot events let alone correct $snapshot events 🙈


This changes the integration tests to look at what was captured and make some assertions on the content.

It wouldn't protect us from that specific bug we can build on this much more easily

@github-actions

github-actions Bot commented Nov 12, 2023 •

Copy link
Copy Markdown
Contributor

Size Change: 0 B

Total Size: 719 kB

ℹ️ View Unchanged
Filename Size
dist/array.full.js 172 kB
dist/array.js 114 kB
dist/es.js 114 kB
dist/exception-autocapture.js 12 kB
dist/module.js 114 kB
dist/recorder-v2.js 95 kB
dist/recorder.js 58.3 kB
dist/surveys.js 39.8 kB

compressed-size-action

@pauldambra
pauldambra requested a review from a team November 12, 2023 20:57
pauldambra added a commit that referenced this pull request Nov 12, 2023
Comment thread cypress/e2e/session-recording.cy.js Outdated
Comment on lines +235 to +236
// even though we trigger 4 events, only 2 snapshots should be captured
// I _think_ this is because Cypress is faking things and they happen too fast

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is more rrweb than cypress. It doesn't capture every single mousemove event, rather it smooths it out and captures sampled movements

@benjackwhite benjackwhite left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good thinking

Comment thread cypress/e2e/session-recording.cy.js Resolved
Comment thread cypress/e2e/session-recording.cy.js Resolved
Comment thread cypress/e2e/session-recording.cy.js Resolved
Comment thread cypress/e2e/session-recording.cy.js Resolved
Comment thread cypress/e2e/session-recording.cy.js Resolved
Comment thread cypress/e2e/session-recording.cy.js Resolved
Comment thread cypress/e2e/session-recording.cy.js Resolved
Comment thread cypress/e2e/session-recording.cy.js Resolved
@pauldambra
pauldambra merged commit 69a078d into master Nov 13, 2023
@pauldambra
pauldambra deleted the fix/meaningful-recordings-integration-tests branch November 13, 2023 08:38
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.

2 participants