Generate Bid ID Test Hardening #3491
Conversation
SyntaxNode
changed the title
| @@ -104,7 +104,7 @@ type bidResponseWrapper struct { | |||
| } | |||
|
|
|||
| type BidIDGenerator interface { | |||
| New() (string, error) | |||
| New(bidder string) (string, error) | |||
There was a problem hiding this comment.
Added a bidder name as input to the generator for deterministic results during test. May be useful if hosts want to add per-bidder logic.
There was a problem hiding this comment.
Generally I'm skeptical about modifying non-test code to enhance tests. It's probably ok in this case.
There was a problem hiding this comment.
I agree with you. I need some way in these tests to get around the random ordering of bidders to fit into the json test framework. I could pull this out into a separate unit test and not need to make this change. I also feel it's probably ok in this case since this isn't a literal "if test, do this" code block and there are potential use cases where an bid generator might want to generate bidder specific ids.
| for bidder, seatBid := range adapterBids { | ||
| for i := range seatBid.Bids { | ||
| if bidID, err := e.bidIDGenerator.New(bidder.String()); err == nil { | ||
| seatBid.Bids[i].GeneratedBidID = bidID |
There was a problem hiding this comment.
Updated to referencing the slice via index instead of an accessor, but this is not a loop bug since the accessor is a pointer.
exchange/exchange_test.go
Outdated
| } | ||
|
|
||
| func (big *mockBidIDGenerator) Enabled() bool { | ||
| return big.GenerateBidID | ||
| func (m *fakeBidIDGenerator) Enabled() bool { |
There was a problem hiding this comment.
Should this variable name be f? I would assume m would be if the generator was still called mock.
There was a problem hiding this comment.
Sure. Change made.
I found no issue with GenerateBidID. Sharing the local changes I made to be sure this feature is working properly.