Skip to content

Test: Multiple biddercode support #3687

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 26 commits into from
Mar 19, 2025
Merged

Test: Multiple biddercode support #3687

merged 26 commits into from
Mar 19, 2025

Conversation

marki1an
Copy link
Collaborator

@marki1an marki1an commented Jan 21, 2025 •
edited
Loading

🔧 Type of changes

  • new bid adapter
  • bid adapter update
  • new feature
  • new analytics adapter
  • new module
  • module update
  • bugfix
  • documentation
  • configuration
  • dependency update
  • tech debt (test coverage, refactorings, etc.)

✨ 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

  • verify email contact works
  • NO fully dynamic hostnames
  • geographic host parameters are NOT required
  • direct use of HTTP is prohibited - implement an existing Bidder interface that will do all the job
  • if the ORTB is just forwarded to the endpoint, use the generic adapter - define the new adapter as the alias of the generic adapter
  • cover an adapter configuration with an integration test

🧪 Test plan

How do you know the changes are safe to ship to production?

🏎 Quality check

  • Are your changes following our code style guidelines?
  • Are there any breaking changes in your code?
  • Does your test coverage exceed 90%?
  • Are there any erroneous console logs, debuggers or leftover code in your changes?

@marki1an marki1an added the tests Functional or other tests label Jan 21, 2025
@marki1an marki1an self-assigned this Jan 21, 2025
@marki1an marki1an added the do not merge Not the time for merging yet label Jan 21, 2025
@marki1an marki1an added the work in progress Signals not finished work label Jan 28, 2025
@marki1an
Merge remote-tracking branch 'refs/remotes/origin/master' into qaa/mu…
1d4e5d6 
…ltiple-biddercode-support

# Conflicts:
#	src/test/groovy/org/prebid/server/functional/model/request/auction/Prebid.groovy
#	src/test/groovy/org/prebid/server/functional/tests/BidderParamsSpec.groovy
@marki1an
Refactor functional tests
560a513
@marki1an marki1an added do not merge Not the time for merging yet and removed work in progress Signals not finished work do not merge Not the time for merging yet labels Feb 13, 2025
@osulzhenko osulzhenko self-requested a review February 13, 2025 14:59
marki1an added 2 commits March 5, 2025 14:39
@marki1an
Merge remote-tracking branch 'refs/remotes/origin/multiple-bidder-cod…
18cde43 
…es' into qaa/multiple-biddercode-support

# Conflicts:
#	src/test/groovy/org/prebid/server/functional/tests/BidderParamsSpec.groovy
@marki1an
Update functional tests
0c04b53
@marki1an marki1an changed the base branch from master to multiple-bidder-codes March 8, 2025 19:37
osulzhenko
osulzhenko requested changes Mar 11, 2025
View reviewed changes
src/test/groovy/org/prebid/server/functional/tests/AlternateBidderCodeSpec.groovy Outdated
given: "Default bid request with alternate bidder codes"
def bidRequest = bidRequestWithAmxBidderAndAlternateBidderCode().tap {
ext.prebid.alternateBidderCodes = requestedAlternateBidderCodes
setAccountId(PBSUtils.randomString)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actual

@osulzhenko osulzhenko removed do not merge Not the time for merging yet blocked labels Mar 11, 2025
osulzhenko
osulzhenko requested changes Mar 18, 2025
View reviewed changes
src/test/groovy/org/prebid/server/functional/tests/AlternateBidderCodeSpec.groovy Outdated
given: "Default bid request with alternate bidder codes"
def bidRequest = bidRequestWithAmxBidderAndAlternateBidderCode().tap {
ext.prebid.alternateBidderCodes = requestedAlternateBidderCodes
setAccountId(PBSUtils.randomString)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actual

osulzhenko
osulzhenko requested changes Mar 18, 2025
View reviewed changes
src/test/groovy/org/prebid/server/functional/tests/BidAdjustmentSpec.groovy Outdated
Comment on lines 1117 to 1118
def bidAdjustment = randomDecimal
def mediaTypeBidAdjustment = bidAdjustmentFactor
Copy link
Collaborator

Choose a reason for hiding this comment

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

redundant variables

src/test/groovy/org/prebid/server/functional/tests/AlternateBidderCodeSpec.groovy Outdated
def "PBS shouldn't discard bid when alternate bidder code allows bidder codes fully configured with different case"() {
given: "Default bid request with amx bidder"
def bidRequest = bidRequestWithAmxBidder().tap {
setAccountId(PBSUtils.randomString)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need it?

src/test/groovy/org/prebid/server/functional/tests/AlternateBidderCodeSpec.groovy Outdated
imp[0].ext.prebid.bidder.generic = null
it.ext.prebid.aliases = [(ALIAS.value): GENERIC]
it.ext.prebid.alternateBidderCodes.bidders = [(ALIAS): new BidderConfig(enabled: true, allowedBidderCodesLowerCase: [GENERIC])]
it.ext.prebid.targeting = new Targeting()
Copy link
Collaborator

Choose a reason for hiding this comment

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

included by bidRequestWithAmxBidderAndAlternateBidderCode

src/test/groovy/org/prebid/server/functional/tests/AlternateBidderCodeSpec.groovy Outdated
amx = null
alias = new Generic()
}
ext.prebid.targeting = new Targeting()
Copy link
Collaborator

Choose a reason for hiding this comment

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

included by bidRequestWithAmxBidderAndAlternateBidderCode

src/test/groovy/org/prebid/server/functional/tests/AlternateBidderCodeSpec.groovy Outdated
}
}

private static BidRequest bidRequestWithAmxBidder() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Methods should be verbs, in mixed case with the first letter lowercase, with the first letter of each internal word capitalized. Same for others
https://www.oracle.com/java/technologies/javase/codeconventions-namingconventions.html
so should be getBidRequestWithAmxBidder or maybe getAmxBidRequest

@osulzhenko osulzhenko merged commit 482bac0 into multiple-bidder-codes Mar 19, 2025
1 check passed
@osulzhenko osulzhenko deleted the qaa/multiple-biddercode-support branch March 19, 2025 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@osulzhenko osulzhenko osulzhenko approved these changes

Assignees

@marki1an marki1an

Labels
tests Functional or other tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@marki1an @osulzhenko