Refactor ApplicationExitInfo to move it to Event.Application.Execution instead of Report. #2668
Conversation
…r and SessionReportingCoordinator
…s and Exception nullable
Macrobenchmark ReportAffected SDKsMeasurements are for head commit (167f2c1). Diffing against base commit (1b56da8) is working in progress.
|
This reverts commit f27bbc7.
| assertEquals(testApplicationExitInfo.getTimestamp(), event.getTimestamp()); | ||
| assertEquals( | ||
| testApplicationExitInfo.getImportance() | ||
| != ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND, |
There was a problem hiding this comment.
makeAppExitInfo is setting ApplicationExitInfo.importance to FOREGROUND. Does that set event.app.background as well?
This condition is hard to read:
testApplicationExitInfo.getImportance() set to IMPORTANCE_FOREGROUND, so: testApplicationExitInfo.getImportance()
!= ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND evaluates false. I am not sure what is the default value for event.getApp().getBackground()
There was a problem hiding this comment.
Cleaned it up to make it more obvious (and added an extra test), but yes, converting an AppExitInfo to Event uses testApplicationExitInfo.getImportance() to decide whether it's foreground or background.
| } | ||
|
|
||
| public Event captureAnrEventData(CrashlyticsReport.ApplicationExitInfo applicationExitInfo) { | ||
| final int orientation = context.getResources().getConfiguration().orientation; |
There was a problem hiding this comment.
nit: this is not the orientation of device at the time of ANR. may be do you want to add a comment here?
|
|
||
| private static Event makeAnrEvent() { | ||
| return makeTestEvent("ANR"); | ||
| return Event.builder() |
There was a problem hiding this comment.
I see similar create ANR event methods in other unit tests as well. would it be possible to create a utility method to create ANR and use that in all the tests?
There was a problem hiding this comment.
The codebase currently doesn't use a utility class and there are a decent number of methods that are repeated across unit tests eg.
- makeTestApplication
- makeTestEvents
etc.
I can take on a task to refactor this in a separate PR!
|
@tejasd: The following tests failed, say
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
4 participants
No description provided.