test(browser): Add integration tests for sessions. #4500
Conversation
| @@ -0,0 +1,11 @@ | |||
| document.getElementById('start-session').addEventListener('click', () => { | |||
| Sentry.getCurrentHub().startSession(); | |||
There was a problem hiding this comment.
I would prefer if we only tested the autoSessionTracking for now, so the sessions that start in pageload/navigation. Let’s not do manual sessions for the time being.
|
|
||
| const session = await page.evaluateHandle<Session>('window.Sentry.getCurrentHub().getScope().getSession()'); | ||
|
|
||
| return session.jsonValue(); |
There was a problem hiding this comment.
I would prefer if we assert on the session envelopes instead of grabbing it this way.
size-limit report
|
| * @return {*} {Promise<SessionContext>} | ||
| */ | ||
| async function getCurrentSession(page: Page, url?: string): Promise<SessionContext> { | ||
| return (await getMultipleSentryTransactionRequests(page, 1, url))[0]; |
There was a problem hiding this comment.
Technically this is not guaranteed to be a session right?
I think we should rename getMultipleSentryTransactionRequests -> getMultipleSentryEnvelopeRequests and directly call that in the tests. The tests can just handle picking the right request and asserting on it (they can even assert on the total amount of requests sent if needed).
Maybe we use getMultipleSentryEnvelopeRequests just for this PR, and then migrate the other test files in another PR.
There was a problem hiding this comment.
Yes, and #4527 seems to solve the confusion. I'll update the PR with proper types after that gets merged, unless this one is urgent?
There was a problem hiding this comment.
Nothing using those types internally yet, I would just make the changes here and we can come back and update this after.
|
@AbhiPrasad, I updated as discussed. Will also open another PR to update the other tests to use the new helper(s). Renaming |
|
I agree with those changes, let’s move forward with that. |
Resolves: #4358
I could not reproduce the case when the session status is not
'ok'.