-
Notifications
You must be signed in to change notification settings - Fork 187
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
Tests: Multiple Uids Cookies Support #3691
Tests: Multiple Uids Cookies Support #3691
Conversation
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.
Add tests for config:
The values can be 1-30. Anything outside of that range should cause a startup failure. If not specified, the default value is 1.
When value -1 and biggest than 30
@@ -16,11 +16,11 @@ class UidsCookie { | |||
Map<BidderName, UidWithExpiry> tempUIDs | |||
Boolean optout | |||
|
|||
static UidsCookie getDefaultUidsCookie(BidderName bidder = GENERIC) { | |||
static UidsCookie getDefaultUidsCookie(BidderName bidder = GENERIC, Integer expireDays = 2) { |
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.
You should choose one name for expiration day(expireDays or plusDays)
@@ -11,10 +11,10 @@ class UidWithExpiry { | |||
String uid | |||
ZonedDateTime expires | |||
|
|||
static UidWithExpiry getDefaultUidWithExpiry() { | |||
static UidWithExpiry getDefaultUidWithExpiry(Integer plusDays = 2) { |
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.
This one is related to the comment above.
@@ -6,7 +6,7 @@ import org.prebid.server.functional.model.UidsCookie | |||
@ToString(includeNames = true, ignoreNulls = true) | |||
class SetuidResponse { | |||
|
|||
Map<String, String> headers | |||
LinkedHashMap<String, List<String>> headers |
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 believe here Map should be fine.
@@ -312,14 +335,50 @@ class SetUidSpec extends BaseSpec { | |||
pbsServiceFactory.removeContainer(pbsConfig) | |||
} | |||
|
|||
def "PBS SetUid should remove most distant expiration bidder from uids cookie in prioritized bidder"() { |
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.
PBS set uid should emit sizedout metric and remove most distant expiration bidder from uids cookie in prioritized bidder
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.
Please ad test for sizeblocked.
assert uidsCookie.tempUIDs[RUBICON] | ||
} | ||
|
||
def "PBS should sout multiple uids cookies by priority and expiration timestamp"() { |
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.
typo: send*
PBS should send multiple uids cookies by priority and expiration timestamp
then: "Response should contain requested uids" | ||
assert response.uidsCookie.tempUIDs[GENERIC] | ||
assert response.uidsCookie.tempUIDs[RUBICON] |
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.
Wrong description
or assert
, don't see check uids
…/multiple-uids-cookies
🔧 Type of changes
✨ What's the context?
What's the context for the changes?
🧠 Rationale behind the change
Why did you choose to make these changes? Were there any trade-offs you had to consider?
🔎 New Bid Adapter Checklist
🧪 Test plan
How do you know the changes are safe to ship to production?
🏎 Quality check