-
-
Notifications
You must be signed in to change notification settings - Fork 440
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
test(critical): Add Mock Relay to verify crash envelope #3811
base: main
Are you sure you want to change the base?
Conversation
…nto kw/test/add-critical-ui-tests
…l/build.gradle.kts Co-authored-by: Stefano <[email protected]>
…ical-tests-matrix
… killing the emulator
This reverts commit d03418e.
This looks quite similar to what we have in #3828 . I went a bit different route where the test just resets relays storage before running and then is able to retrieve all ingested envelopes, parse them and assert in more detail. Maybe we can align what we have. While it should be very simple to port the python mock to kotlin, I guess the python one is a bit easier to run on CI. |
Thanks for linking #3828, I didn't know about it. I went with the Kotlin server purly to use the same lang. The reason why I assert the envelope on the server is that the Maestro Tests are declarative. So if not evaluated on the server, I would have fetch then in the Android app, or create new script which checks the envelopes. |
I'm not opposed to migrate the python mock server over to Kotlin (for the sake of having it in a language, more familiar to maintainers of this repo). I just changed the existing python mock server we have in SAGP a bit.
Gotcha, as long as there's just one simple test, it's probably best to keep it as is for now. I've opened #3830 so we can come back to this at some point as I don't see this as important to address immediately. |
@romtsn @stefanosiano @markushi Can you review this, if it's good from the Android point of view? |
i think it's good, but what's the gain over the current MockRelay we use in UI tests? |
With a stand alone (If I understand the current impl, the okhttp mock runs in the app) MockRelay server we can assert crash envelopes. When the server is mocked inside of the app, we can't update the UI during the crash and the in memory server would have to save its data to disk, otherwise envelopes from before crash would be lost. |
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.
If this works properly, it would be great to build it so that it can be reused across multiple SDKS.
@philipphofmann Yes, shared implementation would be helpful. @stefanosiano Have I understand the Are there any blockers to merge this PR? |
@krystofwoldrich However we talked about the possibility to a shared implementation, starting by merging all current java implementations, and that would come later. I'll give another look, but there shouldn't be any blocker |
@stefanosiano Thank you, yes make sense, we should unify the implementation in |
📜 Description
💡 Motivation and Context
This PR add a mock relay server to verify that the crash envelope is sent
💚 How did you test it?
ci, locally
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps
#skip-changelog