diff --git a/adapters/adapterstest/test_json.go b/adapters/adapterstest/test_json.go index 95319f1e328..35a5a57021a 100644 --- a/adapters/adapterstest/test_json.go +++ b/adapters/adapterstest/test_json.go @@ -7,8 +7,10 @@ import ( "regexp" "testing" + "github.com/mitchellh/copystructure" "github.com/mxmCherry/openrtb/v15/openrtb2" "github.com/prebid/prebid-server/adapters" + "github.com/stretchr/testify/assert" "github.com/yudai/gojsondiff" "github.com/yudai/gojsondiff/formatter" @@ -107,24 +109,10 @@ func runSpec(t *testing.T, filename string, spec *testSpec, bidder adapters.Bidd } else if isVideoTest { reqInfo.PbsEntryPoint = "video" } - actualReqs, errs := bidder.MakeRequests(&spec.BidRequest, &reqInfo) - diffErrorLists(t, fmt.Sprintf("%s: MakeRequests", filename), errs, spec.MakeRequestErrors) - diffHttpRequestLists(t, filename, actualReqs, spec.HttpCalls) - bidResponses := make([]*adapters.BidderResponse, 0) + requests := testMakeRequestsImpl(t, filename, spec, bidder, &reqInfo) - var bidsErrs = make([]error, 0, len(spec.MakeBidsErrors)) - for i := 0; i < len(actualReqs); i++ { - thisBidResponse, theseErrs := bidder.MakeBids(&spec.BidRequest, spec.HttpCalls[i].Request.ToRequestData(t), spec.HttpCalls[i].Response.ToResponseData(t)) - bidsErrs = append(bidsErrs, theseErrs...) - bidResponses = append(bidResponses, thisBidResponse) - } - - diffErrorLists(t, fmt.Sprintf("%s: MakeBids", filename), bidsErrs, spec.MakeBidsErrors) - - for i := 0; i < len(spec.BidResponses); i++ { - diffBidLists(t, filename, bidResponses[i], spec.BidResponses[i].Bids) - } + testMakeBidsImpl(t, filename, spec, bidder, requests) } type testSpec struct { @@ -194,8 +182,8 @@ type expectedBid struct { // // Marshalling the structs and then using a JSON-diff library isn't great either, since -// diffHttpRequests compares the actual http requests to the expected ones. -func diffHttpRequestLists(t *testing.T, filename string, actual []*adapters.RequestData, expected []httpCall) { +// assertMakeRequestsOutput compares the actual http requests to the expected ones. +func assertMakeRequestsOutput(t *testing.T, filename string, actual []*adapters.RequestData, expected []httpCall) { t.Helper() if len(expected) != len(actual) { @@ -206,7 +194,7 @@ func diffHttpRequestLists(t *testing.T, filename string, actual []*adapters.Requ } } -func diffErrorLists(t *testing.T, description string, actual []error, expected []testSpecExpectedError) { +func assertErrorList(t *testing.T, description string, actual []error, expected []testSpecExpectedError) { t.Helper() if len(expected) != len(actual) { @@ -227,10 +215,10 @@ func diffErrorLists(t *testing.T, description string, actual []error, expected [ } } -func diffBidLists(t *testing.T, filename string, response *adapters.BidderResponse, expected []expectedBid) { +func assertMakeBidsOutput(t *testing.T, filename string, bidderResponse *adapters.BidderResponse, expected []expectedBid) { t.Helper() - if (response == nil || len(response.Bids) == 0) != (len(expected) == 0) { + if (bidderResponse == nil || len(bidderResponse.Bids) == 0) != (len(expected) == 0) { if len(expected) == 0 { t.Fatalf("%s: expectedBidResponses indicated a nil response, but mockResponses supplied a non-nil response", filename) } @@ -239,17 +227,15 @@ func diffBidLists(t *testing.T, filename string, response *adapters.BidderRespon } // Expected nil response - give diffBids something to work with. - if response == nil { - response = new(adapters.BidderResponse) + if bidderResponse == nil { + bidderResponse = new(adapters.BidderResponse) } - actual := response.Bids - - if len(actual) != len(expected) { - t.Fatalf("%s: MakeBids returned wrong bid count. Expected %d, got %d", filename, len(expected), len(actual)) + if len(bidderResponse.Bids) != len(expected) { + t.Fatalf("%s: MakeBids returned wrong bid count. Expected %d, got %d", filename, len(expected), len(bidderResponse.Bids)) } - for i := 0; i < len(actual); i++ { - diffBids(t, fmt.Sprintf("%s: typedBid[%d]", filename, i), actual[i], &(expected[i])) + for i := 0; i < len(bidderResponse.Bids); i++ { + diffBids(t, fmt.Sprintf("%s: typedBid[%d]", filename, i), bidderResponse.Bids[i], &(expected[i])) } } @@ -331,3 +317,78 @@ func diffJson(t *testing.T, description string, actual []byte, expected []byte) } } } + +// testMakeRequestsImpl asserts the resulting values of the bidder's `MakeRequests()` implementation +// against the expected JSON-defined results and ensures we do not encounter data races in the process. +// To assert no data races happen we make use of: +// 1) A shallow copy of the unmarshalled openrtb2.BidRequest that will provide reference values to +// shared memory that we don't want the adapters' implementation of `MakeRequests()` to modify. +// 2) A deep copy that will preserve the original values of all the fields. This copy remains untouched +// by the adapters' processes and serves as reference of what the shared memory values should still +// be after the `MakeRequests()` call. +func testMakeRequestsImpl(t *testing.T, filename string, spec *testSpec, bidder adapters.Bidder, reqInfo *adapters.ExtraRequestInfo) []*adapters.RequestData { + t.Helper() + + deepBidReqCopy, shallowBidReqCopy, err := getDataRaceTestCopies(&spec.BidRequest) + assert.NoError(t, err, "Could not create request copies. %s", filename) + + // Run MakeRequests + requests, errs := bidder.MakeRequests(&spec.BidRequest, reqInfo) + + // Compare MakeRequests actual output versus expected values found in JSON file + assertErrorList(t, fmt.Sprintf("%s: MakeRequests", filename), errs, spec.MakeRequestErrors) + assertMakeRequestsOutput(t, filename, requests, spec.HttpCalls) + + // Assert no data races occur using original bidRequest copies of references and values + assert.Equal(t, deepBidReqCopy, shallowBidReqCopy, "Data race found. Test: %s", filename) + + return requests +} + +// getDataRaceTestCopies returns a deep copy and a shallow copy of the original bidRequest that will get +// compared to verify no data races occur. +func getDataRaceTestCopies(original *openrtb2.BidRequest) (*openrtb2.BidRequest, *openrtb2.BidRequest, error) { + cpy, err := copystructure.Copy(original) + if err != nil { + return nil, nil, err + } + deepReqCopy := cpy.(*openrtb2.BidRequest) + + shallowReqCopy := *original + + // Prebid Server core makes shallow copies of imp elements and adapters are allowed to make changes + // to them. Therefore, we need shallow copies of Imp elements here so our test replicates that + // functionality and only fail when actual shared momory gets modified. + if original.Imp != nil { + shallowReqCopy.Imp = make([]openrtb2.Imp, len(original.Imp)) + copy(shallowReqCopy.Imp, original.Imp) + } + + return deepReqCopy, &shallowReqCopy, nil +} + +// testMakeBidsImpl asserts the results of the bidder MakeBids implementation against the expected JSON-defined results +func testMakeBidsImpl(t *testing.T, filename string, spec *testSpec, bidder adapters.Bidder, makeRequestsOut []*adapters.RequestData) { + t.Helper() + + bidResponses := make([]*adapters.BidderResponse, 0) + var bidsErrs = make([]error, 0, len(spec.MakeBidsErrors)) + + // We should have as many bids as number of adapters.RequestData found in MakeRequests output + for i := 0; i < len(makeRequestsOut); i++ { + // Run MakeBids with JSON refined spec.HttpCalls info that was asserted to match MakeRequests + // output inside testMakeRequestsImpl + thisBidResponse, theseErrs := bidder.MakeBids(&spec.BidRequest, spec.HttpCalls[i].Request.ToRequestData(t), spec.HttpCalls[i].Response.ToResponseData(t)) + + bidsErrs = append(bidsErrs, theseErrs...) + bidResponses = append(bidResponses, thisBidResponse) + } + + // Assert actual errors thrown by MakeBids implementation versus expected JSON-defined spec.MakeBidsErrors + assertErrorList(t, fmt.Sprintf("%s: MakeBids", filename), bidsErrs, spec.MakeBidsErrors) + + // Assert MakeBids implementation BidResponses with expected JSON-defined spec.BidResponses[i].Bids + for i := 0; i < len(spec.BidResponses); i++ { + assertMakeBidsOutput(t, filename, bidResponses[i], spec.BidResponses[i].Bids) + } +} diff --git a/go.mod b/go.mod index 585798cbe66..b7fab89fba1 100644 --- a/go.mod +++ b/go.mod @@ -27,6 +27,7 @@ require ( github.com/magiconair/properties v1.8.0 github.com/mattn/go-colorable v0.1.2 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect + github.com/mitchellh/copystructure v1.1.2 github.com/mitchellh/mapstructure v1.0.0 // indirect github.com/mxmCherry/openrtb/v15 v15.0.0 github.com/pelletier/go-toml v1.2.0 // indirect diff --git a/go.sum b/go.sum index 7a38c2f1dd5..3ddeaa2f8cc 100644 --- a/go.sum +++ b/go.sum @@ -73,8 +73,12 @@ github.com/mattn/go-isatty v0.0.8 h1:HLtExJ+uU2HOZ+wI0Tt5DtUDrx8yhUqDcp7fYERX4CE github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= +github.com/mitchellh/copystructure v1.1.2 h1:Th2TIvG1+6ma3e/0/bopBKohOTY7s4dA8V2q4EUcBJ0= +github.com/mitchellh/copystructure v1.1.2/go.mod h1:EBArHfARyrSWO/+Wyr9zwEkc6XMFB9XyNgFNmRkZZU4= github.com/mitchellh/mapstructure v1.0.0 h1:vVpGvMXJPqSDh2VYHF7gsfQj8Ncx+Xw5Y1KHeTRY+7I= github.com/mitchellh/mapstructure v1.0.0/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= +github.com/mitchellh/reflectwalk v1.0.1 h1:FVzMWA5RllMAKIdUSC8mdWo3XtwoecrH79BY70sEEpE= +github.com/mitchellh/reflectwalk v1.0.1/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= github.com/mxmCherry/openrtb/v15 v15.0.0 h1:inLuQ3Bsima9HLB2v6WjbtEFF69SWOT5Dux4QZtYdrw= github.com/mxmCherry/openrtb/v15 v15.0.0/go.mod h1:TVgncsz6MOzbL7lhun1lNuUBzVBlVDbxf9Fyy1TyhZA= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= @@ -123,7 +127,6 @@ github.com/spf13/viper v1.1.0/go.mod h1:A8kyI5cUJhb8N+3pkfONlcEcZbueH6nhAm0Fq7Sr github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1 h1:2vfRuCMp5sSVIDSqO8oNnWJq7mPa6KVP3iPIwFBuy8A= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= @@ -146,7 +149,6 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/net v0.0.0-20180906233101-161cd47e91fd h1:nTDtHvHSdCn1m6ITfMRqtOd/9+7a3s8RBNOZ3eYZzJA= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= @@ -157,10 +159,8 @@ golang.org/x/net v0.0.0-20201202161906-c7110b5ffcbb/go.mod h1:sp8m0HH+o8qH0wwXwY golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e h1:o3PsSEY8E4eXWkXrIP9YJALUkVZqzHJT5DOasTyn8Vs= golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223 h1:DH4skfRX4EBpamg7iV4ZlCpblAHI6s6TDM39bFZumv8= golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190904154756-749cb33beabd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -170,7 +170,6 @@ golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210112080510-489259a85091 h1:DMyOG0U+gKfu8JZzg2UQe9MeaC1X+xQWlAKcRnjxjCw= golang.org/x/sys v0.0.0-20210112080510-489259a85091/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3 h1:cokOdA+Jmi5PJGXLlLllQSgYigAEfHXJAERHVMaCc2k= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= @@ -191,11 +190,9 @@ google.golang.org/protobuf v1.23.0 h1:4MY060fB1DLGMB/7MBTLnwQUY6+F09GEiz6SsrNqyz google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/fsnotify.v1 v1.4.7 h1:xOHLXZwVvI9hhs+cLKq5+I5onOuwQLhQwiu63xxlHs4= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= -gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=