Skip to content
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

EXCH-10878 Native support for OpenX bidder #10

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

sergseven
Copy link

@sergseven sergseven commented Dec 23, 2024

having the PR here to have the checks running

@sergseven sergseven marked this pull request as ready for review December 23, 2024 15:12
@sergseven
Copy link
Author

@@ -29,4 +29,19 @@ public void openrtb2AuctionShouldRespondWithBidsFromOpenx() throws IOException,
// then
assertJsonEquals("openrtb2/openx/test-auction-openx-response.json", response, singletonList("openx"));
}

@Test
public void openrtb2AuctionWithNativeShouldRespondWithBidsFromOpenx() throws IOException, JSONException {
Copy link
Author

@sergseven sergseven Dec 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: we are considering removing this test case since it seems to be duplicated with the unit tests and it looks like this test level is not dedicated to test the features, but integration only

ref: https://github.com/prebid/prebid-server-java/blob/master/docs/developers/code-style.md#bidder-smoke-tests

Copy link

@bukrain bukrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@gmiedlar-ox gmiedlar-ox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to our recent conversation, maybe it would be worth to add a test case verifying what will be returned from makeHttpRequest when there is multi-ad-placement imp in bid request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants