Currency conversion on adapter JSON tests #2071
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
VeronikaSolovei9
previously approved these changes
Nov 8, 2021
|
@thomasdseao This PR makes some changes to the Impactify adpater's JSON tests to be able to test the currency conversion functionality being used in the |
mansinahar
reviewed
Nov 16, 2021
adapters/adapterstest/test_json.go
Outdated
| @@ -103,17 +105,49 @@ func loadFile(filename string) (*testSpec, error) { | |||
| // | |||
| // More assertions will almost certainly be added in the future, as bugs come up. | |||
| func runSpec(t *testing.T, filename string, spec *testSpec, bidder adapters.Bidder, isAmpTest, isVideoTest bool) { | |||
| reqInfo := adapters.ExtraRequestInfo{} | |||
| reqInfo := getTestExtraRequestInfo(t, filename, spec, isAmpTest, isVideoTest) | |||
|
|
|||
There was a problem hiding this comment.
Super nitpick: Please remove the new line
adapters/adapterstest/test_json.go
Outdated
| reqPrebid := reqExt.GetPrebid() | ||
| if reqPrebid != nil && reqPrebid.CurrencyConversions != nil && len(reqPrebid.CurrencyConversions.ConversionRates) > 0 { | ||
| err = currency.ValidateCustomRates(reqPrebid.CurrencyConversions) | ||
| assert.NoError(t, err, "Error unmarshalling test currency rates. %s", filename) |
There was a problem hiding this comment.
This doesn't have to be a unmarshalling error so please consider rewording this message to the following:
assert.NoError(t, err, "Error validating currency rates in the test request: %s", filename)
VeronikaSolovei9
approved these changes
Nov 18, 2021
|
@mansinahar Everything looks good 👍 |
mansinahar
approved these changes
Nov 18, 2021
Thank you @thomasdseao |
wwwyyy
pushed a commit
to wwwyyy/prebid-server
that referenced
this pull request
Dec 20, 2021
* 'master' of https://github.com/wwwyyy/prebid-server: (23 commits) Add GPID Reserved Bidder Name (prebid#2107) Marsmedia adapter - param zoneId can get string and int (prebid#2105) Algorix: add Server Region Support (prebid#2096) New Adapter: Bizzclick (prebid#2056) Collect Prometheus Go process metrics (prebid#2101) Added channel support (prebid#2037) Rubicon: update fpd fields resolving (prebid#2091) Beachfront - Currency conversion (prebid#2078) New Adapter: Groupm (Alias Of PubMatic) (prebid#2042) Adform adapter lacked gross/net parameter support (prebid#2084) add iframe sync for openx (prebid#2094) changes to the correct user sync url (prebid#2092) Smaato: Add instl to tests (prebid#2089) Correct MakeRequests() output assertion in adapter JSON tests (prebid#2087) Adview: adapter currency fix. (prebid#2060) New Adapter: Coinzilla (prebid#2074) Syncer Level ExternalURL Override (prebid#2072) 33Across: Enable Support for SRA requests (prebid#2079) Currency conversion on adapter JSON tests (prebid#2071) New Adapter: VideoByte (prebid#2058) ...
jorgeluisrocha
pushed a commit
to jwplayer/prebid-server
that referenced
this pull request
Sep 28, 2022
Co-authored-by: Gus Carreon
shunj-nb
pushed a commit
to ParticleMedia/prebid-server
that referenced
this pull request
Nov 8, 2022
Co-authored-by: Gus Carreon
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request #1901 granted adapters the ability to convert currency inside their
MakeRequests(request *openrtb2.BidRequest, reqInfo *adapters.ExtraRequestInfo)implementation. This pull request makes possible for adapters to include their own currency conversion rates in their JSON tests in order to better assert #1901's featuresAlso commited in this PR are JSON tests for the impactify adapter. Impactify's implementation of
MakeRequests(request *openrtb2.BidRequest, reqInfo *adapters.ExtraRequestInfo)makes use of currency conversion and the additions toadapters/impactify/impactifytest/exemplary/sample_banner.jsonassert the correct conversion fo the impression bid floor, and on the other handadapters/impactify/impactifytest/supplemental/currency_rate_not_found.jsonasserts a currency conversion error scenario.@thomasdseao we'd like to hear your take on the tests added to your adapter. Please let us know if you approve.