Tests: Support GET interface profiles#4239
Tests: Support GET interface profiles#4239osulzhenko wants to merge 16 commits intoget-interfacefrom
Conversation
marki1an
left a comment
marki1an
left a comment
There was a problem hiding this comment.
Please add validation for get parameters
| @JsonProperty("sarid") | ||
| String storedAuctionResponseId | ||
|
|
||
| @JsonProperty("mimes") |
There was a problem hiding this comment.
No, really needed here JsonProperty
| @JsonProperty("oh") | ||
| Integer originalHeight | ||
|
|
||
| @JsonProperty("sizes") |
| @JsonProperty("ms") | ||
| Object sizesLegacy | ||
|
|
||
| @JsonProperty("slot") |
There was a problem hiding this comment.
Check for similar occurrences
| List<Integer> api | ||
|
|
||
| @JsonProperty("battr") | ||
| List<Integer> battr |
There was a problem hiding this comment.
Better naming for it blockAttributes
| Integer podSequence | ||
|
|
||
| @JsonProperty("proto") | ||
| List<Integer> proto |
There was a problem hiding this comment.
Please rename to protocols
| and: "Bidder request should contain mimes from param" | ||
| def bidderRequest = bidder.getBidderRequest(request.id) | ||
| assert bidderRequest.imp.size() == 1 | ||
| assert bidderRequest.imp.first.getProperty(impMediaType.value).mimes == [mimes] |
There was a problem hiding this comment.
I'd like to propose creating a utility method to get this field, since we will often use it, and it will be faster than using getProperty.
There was a problem hiding this comment.
Add in where block null value when mimes not defined.
Comment applicable to all tests
| impMediaType << [ | ||
| MediaType.BANNER, | ||
| MediaType.VIDEO, | ||
| MediaType.AUDIO | ||
| ] |
| it.mimes = [mimes] | ||
| } | ||
|
|
||
| "Default stored request" |
| } | ||
| } | ||
|
|
||
| def "PBS should apply ow and oh for banner imp from general get request when it's specified"() { |
There was a problem hiding this comment.
You can merge with PBS should apply width and height for banner imp from general get request when it's specified
| and: "Default bid response" | ||
| def bidResponse = BidResponse.getDefaultBidResponse(request) | ||
| bidder.setResponse(request.id, bidResponse) |
There was a problem hiding this comment.
You can remove bid response since it's present in BaseSpec
There was a problem hiding this comment.
Please take a look at this comment
| and: "Default bid response" | ||
| def bidResponse = BidResponse.getDefaultBidResponse(request) | ||
| bidder.setResponse(request.id, bidResponse) |
There was a problem hiding this comment.
Please take a look at this comment
| given: "Default General get request" | ||
| def validMemis = PBSUtils.randomString | ||
| def generalGetRequest = GeneralGetRequest.default.tap { | ||
| it.mimes = (invalidMimes + validMemis) |
There was a problem hiding this comment.
What about "$invalid $valid"?
|
|
||
| def "PBS should apply width and height for banner imp from general get request when banner width and height are square dimensions"() { | ||
| given: "Default General get request" | ||
| def side = PBSUtils.getRandomNumber(0, 10) |
|
|
||
| def "PBS should apply sizes banner imp from general get request when banner width and height are square dimensions"() { | ||
| given: "Default General get request" | ||
| def side = PBSUtils.getRandomNumber(0, 10) |
| impMediaType << [ | ||
| MediaType.VIDEO, | ||
| MediaType.AUDIO | ||
| ] |
There was a problem hiding this comment.
| impMediaType << [ | |
| MediaType.VIDEO, | |
| MediaType.AUDIO | |
| ] | |
| impMediaType << [MediaType.VIDEO, MediaType.AUDIO] |
| ] | ||
| } | ||
|
|
||
| def "PBS should apply api from general get request when it's specified"() { |
There was a problem hiding this comment.
It would be nice to see all tests with a negative scenario when not specified
src/test/groovy/org/prebid/server/functional/tests/GeneralGetInterfaceImpSpec.groovy
Show resolved
Hide resolved
| assert bidderRequest.imp.first.video.linearity == linearityParam | ||
| } | ||
|
|
||
| def "PBS should apply minbr from general get request when it's specified"() { |
There was a problem hiding this comment.
PBS should apply minbr and maxbr from general get request when it's specified
| assert !response.ext?.errors | ||
| assert !response.ext?.warnings | ||
|
|
||
| and: "Bidder request should contain boxingallowed from param" |
| assert !response.ext?.errors | ||
| assert !response.ext?.warnings | ||
|
|
||
| and: "Bidder request should contain boxingallowed from param" |
| storedRequestDao.save(storedRequest) | ||
|
|
||
| when: "PBS processes general get request" | ||
| def response = pbsWithStoredProfiles.(getRequest) |
| when: "PBS processes amp request" | ||
| defaultPbsService.sendGeneralGetRequest(generalGetRequest) | ||
|
|
||
| then: "Amp response should contain value from targeting in imp.ext.data" |
| } | ||
|
|
||
| def "PBS should throw exception when profiles are not configured for filesystem and request contain profileId"() { | ||
| def "PBS should process request when profiles are not configured for filesystem and request contain profileId"() { |
There was a problem hiding this comment.
process request with a warning
.../groovy/org/prebid/server/functional/model/request/auction/ConsentedProvidersSettings.groovy
Show resolved
Hide resolved
src/test/groovy/org/prebid/server/functional/model/request/auction/Device.groovy
Show resolved
Hide resolved
| assert !response.ext?.errors | ||
| assert !response.ext?.warnings | ||
|
|
||
| and: "Bidder request should contain addtlConsent from param" |
There was a problem hiding this comment.
"Bidder request should contain regs.gpp from param"
| it.ext == new RegsExt() | ||
| } | ||
|
|
||
| and: "PBS should cansel request" |
| assert !response.ext?.errors | ||
| assert !response.ext?.warnings | ||
|
|
||
| and: "Bidder request should be valid" |
There was a problem hiding this comment.
"PBs should preform bidder request"
| assert !response.ext?.errors | ||
| assert !response.ext?.warnings | ||
|
|
||
| and: "Bidder request should contain outputformat from param" |
There was a problem hiding this comment.
"Bidder request should contain outputModule from param"
| assert !response.ext?.errors | ||
| assert !response.ext?.warnings | ||
|
|
||
| and: "Bidder request should contain gpc from param" |
There was a problem hiding this comment.
"Bidder request should contain dnt from param"
osulzhenko
force-pushed
the
functional-tests/general-get-interface
branch
from
934cb46 to
5056fdd
Compare
| String result = value.join(',') | ||
| generator.writeString(result) |
There was a problem hiding this comment.
In line?
generator.writeString(value.join(','))
| private Response getAuction(GeneralGetRequest request, Map<String, String> headers = [:]) { | ||
| def map = toMap(request) | ||
|
|
||
| given(requestSpecification).headers(headers) | ||
| .queryParams(map) | ||
| .get(GENERAL_GET_ENDPOINT) | ||
| } |
There was a problem hiding this comment.
Same here
given(requestSpecification).headers(headers)
.queryParams(toMap(request))
.get(GENERAL_GET_ENDPOINT)
| and: "Shouldn't contain ext privacy info" | ||
| assert bidderRequest.regs.ext == new RegsExt() |
There was a problem hiding this comment.
Naming: "Bidder request shouldn't contain regs.ext"
Please replace with assert bidderRequest.regs.ext
There was a problem hiding this comment.
It's better to assert against new RegsExt(). Otherwise, the assertion could be true for any non-null value instead of specifically an empty object
| ] | ||
| } | ||
|
|
||
| @PendingFeature |
| } | ||
|
|
||
| @PendingFeature | ||
| def "PBS should emit error when consent type is invalid"() { |
There was a problem hiding this comment.
PBS get interface should emit error when consent type is invalid
| } | ||
|
|
||
| and: "Default stored request" | ||
| def request = BidRequest.getDefaultBidRequest().tap { |
There was a problem hiding this comment.
variable name storedRequest is inaccurate because it is actually a bidRequest. Additionally, we are already using storedRequest for variable immediately following this one
| and: "Response should contain error" | ||
| assert !response.ext?.errors |
| assert response.ext?.warnings[ErrorType.PREBID]*.code == [999] | ||
| assert response.ext?.warnings[ErrorType.PREBID]*.message == [NO_PROFILE_MESSAGE.formatted(requestProfile.id)] | ||
|
|
||
| and: "Response should contain error" |
| assert bidder.getBidderRequest(request.id).imp.banner == request.imp.banner | ||
| } | ||
|
|
||
| def "PBS should prioritise profile for request and emit warning when profile included as parameter more than limit"() { |
There was a problem hiding this comment.
where warning in test case?
| impMediaType << [MediaType.BANNER, MediaType.VIDEO, MediaType.AUDIO] | ||
| } | ||
|
|
||
| @PendingFeature |
2 participants
🔧 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