-
-
Notifications
You must be signed in to change notification settings - Fork 468
feat(replay): Add ReplaySnapshotObserver for snapshot testing #5386
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
base: main
Are you sure you want to change the base?
Changes from all commits
d49b64a
4b5fb3a
9d6f12a
240dd96
1f6b03c
d2b2259
2b576be
f2c0c49
f39d8f5
1720230
4a829fe
23af62e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,4 +32,5 @@ artifacts: | |
| when: always | ||
| match: | ||
| - junit.xml | ||
| - "*.png" | ||
| directory: ./artifacts/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| package io.sentry.uitest.android | ||
|
|
||
| import android.graphics.Bitmap | ||
| import android.os.Environment | ||
| import androidx.lifecycle.Lifecycle | ||
| import androidx.test.core.app.launchActivity | ||
| import io.sentry.SentryReplayOptions | ||
| import io.sentry.TypeCheckHint | ||
| import java.io.File | ||
| import java.util.concurrent.CopyOnWriteArraySet | ||
| import java.util.concurrent.CountDownLatch | ||
| import java.util.concurrent.TimeUnit | ||
| import kotlin.test.Test | ||
| import kotlin.test.assertTrue | ||
| import org.hamcrest.CoreMatchers.`is` | ||
| import org.junit.Assume.assumeThat | ||
| import org.junit.Before | ||
|
|
||
| class ReplaySnapshotTest : BaseUiTest() { | ||
|
|
||
| @Before | ||
| fun setup() { | ||
| // GH Actions emulators don't support capturing screenshots for replay | ||
| @Suppress("KotlinConstantConditions") | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I copied this from |
||
| assumeThat(BuildConfig.ENVIRONMENT != "github", `is`(true)) | ||
| } | ||
|
|
||
| @Test | ||
| fun captureComposeReplayFrameSnapshots() { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just FYI, this is a pretty contrived test but it just makes sure we can capture a replay using the new snapshot observer api |
||
| val snapshotsDir = | ||
| File( | ||
| Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS), | ||
| "sauce_labs_custom_screenshots", | ||
| ) | ||
| .apply { | ||
| deleteRecursively() | ||
| mkdirs() | ||
| } | ||
| val frameReceived = CountDownLatch(1) | ||
| val capturedScreens = CopyOnWriteArraySet<String>() | ||
|
|
||
| val activityScenario = launchActivity<ComposeActivity>() | ||
| activityScenario.moveToState(Lifecycle.State.RESUMED) | ||
|
|
||
| initSentry { | ||
| it.sessionReplay.sessionSampleRate = 1.0 | ||
| it.sessionReplay.snapshotObserver = | ||
| SentryReplayOptions.ReplaySnapshotObserver { hint, frameTimestamp, screenName -> | ||
| val bitmap = | ||
| hint.getAs(TypeCheckHint.REPLAY_FRAME_BITMAP, Bitmap::class.java) | ||
| ?: return@ReplaySnapshotObserver | ||
| val name = screenName ?: "unknown" | ||
| if (capturedScreens.add(name)) { | ||
| val file = File(snapshotsDir, "${name}_$frameTimestamp.png") | ||
| file.outputStream().use { out -> bitmap.compress(Bitmap.CompressFormat.PNG, 100, out) } | ||
| } | ||
| bitmap.recycle() | ||
| frameReceived.countDown() | ||
| } | ||
| } | ||
|
|
||
| assertTrue(frameReceived.await(10, TimeUnit.SECONDS), "Expected at least one replay frame") | ||
| assertTrue(capturedScreens.isNotEmpty(), "Expected at least one screen captured") | ||
|
|
||
| val files = snapshotsDir.listFiles()?.filter { it.extension == "png" } ?: emptyList() | ||
| assertTrue(files.isNotEmpty(), "Expected snapshot PNG files on disk") | ||
| assertTrue(files.all { it.length() > 0 }, "Snapshot files should not be empty") | ||
|
|
||
| activityScenario.moveToState(Lifecycle.State.DESTROYED) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,6 +61,8 @@ android { | |
|
|
||
| buildFeatures { buildConfig = true } | ||
|
|
||
| configurations.all { resolutionStrategy.force(libs.jetbrains.annotations.get()) } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I need to dig deeper in to why we need to add this to all the modules that add |
||
|
|
||
| androidComponents.beforeVariants { | ||
| it.enable = !Config.Android.shouldSkipDebugVariant(it.buildType) | ||
| } | ||
|
|
@@ -71,6 +73,7 @@ kotlin { explicitApi() } | |
| dependencies { | ||
| api(projects.sentry) | ||
|
|
||
| compileOnly(libs.jetbrains.annotations) | ||
| compileOnly(libs.androidx.compose.ui.replay) | ||
| implementation(kotlin(Config.kotlinStdLib, Config.kotlinStdLibVersionAndroid)) | ||
| // tests | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ import android.view.MotionEvent | |
| import io.sentry.Breadcrumb | ||
| import io.sentry.DataCategory.All | ||
| import io.sentry.DataCategory.Replay | ||
| import io.sentry.Hint | ||
| import io.sentry.IConnectionStatusProvider.ConnectionStatus | ||
| import io.sentry.IConnectionStatusProvider.ConnectionStatus.DISCONNECTED | ||
| import io.sentry.IConnectionStatusProvider.IConnectionStatusObserver | ||
|
|
@@ -17,8 +18,10 @@ import io.sentry.ReplayBreadcrumbConverter | |
| import io.sentry.ReplayController | ||
| import io.sentry.SentryIntegrationPackageStorage | ||
| import io.sentry.SentryLevel.DEBUG | ||
| import io.sentry.SentryLevel.ERROR | ||
| import io.sentry.SentryLevel.INFO | ||
| import io.sentry.SentryOptions | ||
| import io.sentry.TypeCheckHint | ||
| import io.sentry.android.replay.ReplayState.CLOSED | ||
| import io.sentry.android.replay.ReplayState.PAUSED | ||
| import io.sentry.android.replay.ReplayState.RESUMED | ||
|
|
@@ -308,6 +311,20 @@ public class ReplayIntegration( | |
| var screen: String? = null | ||
| scopes?.configureScope { screen = it.screen?.substringAfterLast('.') } | ||
| captureStrategy?.onScreenshotRecorded(bitmap) { frameTimeStamp -> | ||
| val observer = options.sessionReplay.snapshotObserver | ||
| if (observer != null) { | ||
| val copy = bitmap.copy(bitmap.config!!, false) | ||
|
cursor[bot] marked this conversation as resolved.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we copy the bitmap now
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should have our
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the only types of errors that can happen when copying the bitmap are memory errors and we shouldn't try to catch those.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Newbie question, but are we able to observe the performance hit of copying on our end? Esp if we want to expand our use-cases beyond testing or debugging at some point, it'd be good to know whether we need to optimize.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Memory-wise, each bitmap is roughly 900kb and we produce one frame per second. |
||
| if (copy != null) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no point in calling the API if we have a null bitmap |
||
| try { | ||
| val hint = Hint() | ||
| hint.set(TypeCheckHint.REPLAY_FRAME_BITMAP, copy) | ||
| observer.onSnapshotCaptured(hint, frameTimeStamp, screen) | ||
| } catch (e: Throwable) { | ||
| options.logger.log(ERROR, "Error in ReplaySnapshotObserver", e) | ||
| copy.recycle() | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would not have thought of calling copy.recycle() if we get an exception here. AI is smart. |
||
| } | ||
| } | ||
| } | ||
| addFrame(bitmap, frameTimeStamp, screen) | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❗- It's not in the diff, so apologies for missing it the first time round, but I see there's an |
||
| checkCanRecord() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth noting that this is for use in snapshot tests and for debugging purposes and isn't currently optimized for production use.