RUMM-1121 use sensible defaults in Configuration#521
Conversation
| return true | ||
| } | ||
|
|
||
| override fun hashCode(): Int { |
There was a problem hiding this comment.
is there a reason why other classes of this PR have javaClass.hashCode() as body of that method, and this class takes a different approach?
There was a problem hiding this comment.
I think the reason why is that there's no other field in those classes to based your hashcode function on.
There was a problem hiding this comment.
As Marius said, only classes with no field use the javaClass.hashCode() value. When a class has fields (participating in the equals implementation), the same fields need to participate to the hashCode implementation by contract.
| } | ||
|
|
||
| override fun hashCode(): Int { | ||
| return thresholdMs.hashCode() |
There was a problem hiding this comment.
why only this field participates in the hashcode calculation?
There was a problem hiding this comment.
It's because the thresholdNS is derived from thresholdMs so it would be redundant by design, and the startUptimeNs and target are transient property that don't change the strategy's configuration.
|
|
||
| // region Object | ||
|
|
||
| override fun equals(other: Any?): Boolean { |
There was a problem hiding this comment.
why do you need to change the default implementation here ?
There was a problem hiding this comment.
Because the default equals implementation simply compares the objects address.
|
|
||
| // region Object | ||
|
|
||
| override fun equals(other: Any?): Boolean { |
There was a problem hiding this comment.
I would really advise to properly test all these re-implementations of equals/hashcode methods ?
| other as ActivityViewTrackingStrategy | ||
|
|
||
| if (trackExtras != other.trackExtras) return false | ||
| if (componentPredicate != other.componentPredicate) return false |
There was a problem hiding this comment.
here you want to compare by value or by reference ? I think it should be by reference (!==)
There was a problem hiding this comment.
No I want to compare by value
| return true | ||
| } | ||
|
|
||
| override fun hashCode(): Int { |
There was a problem hiding this comment.
I think the reason why is that there's no other field in those classes to based your hashcode function on.
d8f73b6 to
4dc3fa4
Compare
| keepStartingWith("com.android.internal.util.") | ||
| keepStartingWith("android.util.") | ||
| keep("android.content.ComponentName") | ||
| keep("android.os.Looper") |
There was a problem hiding this comment.
Because the tests are using the Loopers.
4dc3fa4 to
ff4430c
Compare
| */ | ||
| @Suppress("ReplaceCallWithBinaryOperator", "StringLiteralDuplication") | ||
| @ExtendWith(ForgeExtension::class) | ||
| abstract class ObjectTest<T : Any> { |
| // When | ||
| repeat(64) { | ||
| val y = createUnequalInstance(x, forge) | ||
| if (y != null && y !== x) { |
There was a problem hiding this comment.
why do you need this comparison ? Can the createUnequalInstace function return the same instance ?
There was a problem hiding this comment.
Because createUnequalInstance is implemented by a child class, I just wanted to be safe.
3 participants
What does this PR do?
Use sensible defaults in RUM Configuration
Motivation
The goal is to make the onboarding quick and easy for new customers with sensible defaults that should:
This means that we set the default ViewTrackingStrategy to track activity (all Android apps will have at least one activity); long tasks are tracked with a default threshold of 100ms (>6 times the threshold for a 60 FPS target) and tracking actions with the default tracker.