-
Notifications
You must be signed in to change notification settings - Fork 179
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
IX: fix ix adapter handling of paapi config #3563
base: master
Are you sure you want to change the base?
IX: fix ix adapter handling of paapi config #3563
Conversation
@Test | ||
public void openrtb2AuctionShouldRespondWithBidsFromIxForPrivacySandboxRequest() throws IOException, JSONException { | ||
// given | ||
WIRE_MOCK_RULE.stubFor(post(urlPathEqualTo("/ix-exchange")) | ||
.withRequestBody(equalToJson(jsonFrom("openrtb2/ix/privacy-sandbox-test-ix-bid-request.json", | ||
prebidVersionProvider))) | ||
.willReturn(aResponse().withBody(jsonFrom("openrtb2/ix/privacy-sandbox-test-ix-bid-response.json")))); | ||
|
||
// when | ||
final Response response = responseFor("openrtb2/ix/privacy-sandbox-test-auction-ix-request.json", | ||
Endpoint.openrtb2_auction); | ||
|
||
// then | ||
assertJsonEquals("openrtb2/ix/privacy-sandbox-test-auction-ix-response.json", response, singletonList("ix")); | ||
} |
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.
do not need to create a separate integration test and a separate set of the json files, just use your json files for the existing test
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.
Do you mean keeping only the Privacy Sandbox scenario test and deleting the regular one? I’d say having both makes sense, as the intention behind these two tests is different—one tests the Privacy Sandbox request scenario, while the other covers a regular (non-Privacy Sandbox) case.
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.
Our integration tests are mainly for the bidder configuration verification. But, since your test case extends the regular one, I suggest using yours payloads instead. So please replace existing JSON payloads with yours.
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.
Ok, updated the base case.
🔧 Type of changes
✨ What's the context?
This is the fix of paapi auction config handling for IX Adapter as described in #3562
🧠 Rationale behind the change
Why did you choose to make these changes? Were there any trade-offs you had to consider?
This change aligns with how Index Exchange formats the auction configuration for the PAAPI response.
🔎 New Bid Adapter Checklist
🧪 Test plan
How do you know the changes are safe to ship to production?
🏎 Quality check