-
Notifications
You must be signed in to change notification settings - Fork 185
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: Transition PAAPI parameters
#3634
base: feature/paa
Are you sure you want to change the base?
Conversation
Corresponding issue prebid/prebid-server#3536 |
then: "Bidder request should imp[].{ae/ext.igs.ae} same value as requested" | ||
def bidderRequest = bidder.getBidderRequest(bidRequest.id) | ||
assert bidderRequest.imp[0].ext.ae == auctionEnvironment | ||
assert bidderRequest.imp[0].ext.interestGroupAuctionSupport.auctionEnvironment == auctionEnvironment |
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.
Comment and code are mismatched
@@ -23,6 +24,7 @@ class AccountAuctionConfig { | |||
AccountEventsConfig events | |||
AccountPriceFloorsConfig priceFloors | |||
Targeting targeting | |||
PaaFormant paaformat |
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.
Should be PaaFormant paaFormat, as in the request variable
UNKNOWN(PBSUtils.getRandomNumberWithExclusion([NOT_SUPPORTED.value, | ||
DEVICE_ORCHESTRATED.value, | ||
SERVER_ORCHESTRATED.value])), |
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 UNKNOWN(Integer.MAX_VALUE)
should be enough
@@ -23,10 +23,12 @@ class ImpExt { | |||
String tid | |||
String gpid | |||
String sid | |||
Integer ae | |||
AuctionEnvironment ae |
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 prefer not to violate Java conventions and to use full names; in this case, it is easy to understand purpose and context
@JsonProperty("ae")
AuctionEnvironment auctionEnvironment
@@ -1040,4 +1046,66 @@ class BidderParamsSpec extends BaseSpec { | |||
assert seatNonBid.nonBid[0].impId == bidRequest.imp[0].id | |||
assert seatNonBid.nonBid[0].statusCode == REQUEST_BLOCKED_UNACCEPTABLE_CURRENCY | |||
} | |||
|
|||
def "PBS should pass imp.ext.ae to imp.ext.igs.ae when imp.ext.ae specified and imp.ext.igs.ae doesn't specified"() { |
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 should add auction environment to imp.ext.igs when it is present in imp.ext and imp.ext.igs is empty
def bidRequest = BidRequest.defaultBidRequest.tap { | ||
imp[0].ext.ae = DEVICE_ORCHESTRATED | ||
imp[0].ext.prebid.bidder.openx = Openx.defaultOpenx | ||
ext.prebid.paaFormat = requestPaaFormant | ||
} |
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 can be a private method for creating a proper request
assert auctionConfigs[0].impId == impId | ||
assert auctionConfigs[0].bidder == bidResponse.seatbid[0].seat.value | ||
assert auctionConfigs[0].adapter == bidResponse.seatbid[0].seat.value | ||
assert auctionConfigs[0].config == fledgeConfig |
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.
auctionConfigs.impId == [impId]
auctionConfigs.bidder == bidResponse.seatbid.seat.value
auctionConfigs.adapter == bidResponse.seatbid.seat.value
auctionConfigs.config == [fledgeConfig]
|
||
and: "PBS shouldn't emit metric" | ||
def metricsRequest = pbsService.sendCollectedMetricsRequest() | ||
assert !metricsRequest["Some name of metric for dropped igb object"] |
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.
Move these stubs to a constant
assert !bidResponse.ext.warnings[PREBID] | ||
|
||
and: "PBS shouldn't emit log" | ||
assert !pbsService.getLogsByValue("DROP igb obj") |
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.
Move these stubs to a constant
|
||
and: "Alert.general metric should be updated" | ||
def metricsRequest = pbsService.sendCollectedMetricsRequest() | ||
assert metricsRequest["alerts.general" as String] == 1 |
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.
assert metrics[ALERT_GENERAL.getValue()] == 1
I noticed that on my PR no caching is taking place.
Required update |
…ests/transition-PAAPI-parameters
…transition-PAAPI-parameters
🔧 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