Capture SeatNonBids for rejected creatives due to insecurity and invalid size #3376
Conversation
ashishshinde-pubm
changed the title
There was a problem hiding this comment.
The tests in this file look good. However, I think it might make sense to add at least one seat nonbid JSON test in exchange/exchangetest to ensure that seat nonbids registered from makeBids are actually added to the response. At first glance, I'm not sure if an update will be needed in the JSON test framework to accommodate, though it does appear seat nonbid changes were made to it as part of the initial seat nonbid PR so maybe you could leverage them. It's also possible you could modify an existing JSON test like
exchange/exchangetest/bid_response_validation_enforce_creative.json instead of creating a new one if that makes sense.
There was a problem hiding this comment.
Added the seatNonBid in existing exchange/exchangetest/bid_response_validation_enforce_one_bid_rejected.json file.
There was a problem hiding this comment.
This looks good, close to approving. I think in line with what @bsardo said, could you also add a JSON test for the other new 352 status code?
|
|
||
| assert.Equal(t, 0, len(resultingErrs), "%s. Test should not return errors \n", test.description) | ||
| assert.Equal(t, test.expectedNumOfBids, len(resultingBids), "%s. Test returns more valid bids than expected\n", test.description) | ||
| assert.Equal(t, test.expectedNonBids, nonBids, "%s. Test returns incorrect nonBids\n", test.description) | ||
| } | ||
| } |
There was a problem hiding this comment.
Nitpick: can you please wrap the tests execution in t.Run function?
Like this:
//Run tests
for _, test := range testCases {
t.Run(test.description, func(t *testing.T) {
e.bidValidationEnforcement = test.givenValidations
sampleBids := test.givenBids
nonBids := &nonBids{}
resultingBids, resultingErrs := e.makeBid(sampleBids, sampleAuction, true, ImpExtInfoMap, bidExtResponse, test.givenSeat, "", nonBids)
assert.Equal(t, 0, len(resultingErrs), "%s. Test should not return errors \n", test.description)
assert.Equal(t, test.expectedNumOfBids, len(resultingBids), "%s. Test returns more valid bids than expected\n", test.description)
assert.Equal(t, test.expectedNonBids, nonBids, "%s. Test returns incorrect nonBids\n", test.description)
})
}This will follow genaral test approach and also allow per-test-case execution.
This PR capture the seatNonBids when bids are rejected due to following reasons -
Refer issue - #2852