-
Notifications
You must be signed in to change notification settings - Fork 32
MutableAttributes unit tests #1279
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
MutableAttributes unit tests #1279
Conversation
Signed-off-by: Tomas Chladek <[email protected]>
...common/src/main/java/com/splunk/rum/integration/agent/common/attributes/MutableAttributes.kt
Show resolved
Hide resolved
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.
lgtm
...common/src/main/java/com/splunk/rum/integration/agent/common/attributes/MutableAttributes.kt
Outdated
Show resolved
Hide resolved
|
||
mutableAttributes.update { | ||
put("updatedKey", "updatedValue") | ||
remove(AttributeKey.stringKey("initialKey")) |
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.
For testing update, we should also try to update the value of key initialValue
instead of removing it. You can add another key that gets removed.
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.
I added a test for this.
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.
Actually I meant, we should add it here - in the update
method test - a test for updating value
of an existing key
.
For example:
val mutableAttributes = MutableAttributes()
mutableAttributes["initialKeyTobeUpdated"] = "initialValueTobeUpdated"
mutableAttributes["initialKeyTobeRemoved"] = "initialValueTobeRemoved"
mutableAttributes.update {
put("updatedKey", "updatedValue")
put("initialKeyTobeUpdated", "newUpdatedValue")
remove(AttributeKey.stringKey("initialKeyTobeRemoved"))
put("anotherKey", 12345L)
}
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.
@surbhiia Tom is on PTO now. I’ll go ahead and merge this, but I’ll let him know and ask him to update it separately. Thanks for the review and approval!
Signed-off-by: Tomas Chladek <[email protected]>
Approved PR, but do see if you can add this - #1279 (comment) |
MutableAttributes unit tests and fixes