Fix adapter JSON tests to have the right test structure

[mansinahar]

We recently discovered that the RunJSONBidderTest function, responsible for running the adapter specific JSON tests does not complain if the JSON test doesn't follow the required structure and we also found that a lot of the adapters weren't actually following the required struct, which meant those bid responses weren't actually getting asserted while running tests.

This PR fixes the JSON tests for all adapters except Beachfront (Beachfront is addressed in #1578)

I updated the RunJSONBidderTest function minimally on my local machine to actually complain when a JSON test doesn't adhere with the required structure and fixed them accordingly. However, I am not pushing the RunJSONBidderTest changes as part of this PR as it will likely be harder to review and so will push those in a follow-up PR.

[bsardo]

This is looking good, just some nitpicks.

[bsardo]

Should this be "impid": "some_test_ad_id",?

[mansinahar]

That's a good question but looks their adapter modifies the impid in the bid to id
Ref: https://github.com/prebid/prebid-server/blob/master/adapters/beintoo/beintoo.go#L206

[bsardo]

Just curious, are you putting every element of a json array on its own line by choice or is that something that is happening automatically via a json formatter?
IMO small lists of integers are easier to read if it's all on one line. I'm not saying we need to revert these type of changes; I'd just like to understand our stance.

[mansinahar]

Yeah, I agree with you. Small lists like this can be on their own line. This for me happened automatically via a JSON formatter. I tried to play around with it's settings to see if I can configure it to not do that but didn't have much luck. Changed it manually for this file but there could be others that still have such lists auto-formatted.

If you know of a VSCode JSON formatter where I can control this, please lemme know. I will be happy to have that in my setup and will also update all the other files.

[bsardo]

Which formatter are you using?

[SyntaxNode]

I use Beautify.

[guscarreon]

Looks good to me just one suggestion: In order to avoid future adapters from misspelling one of the json fields ("mockBidRequest", "httpCalls", "expectedBidResponses", "expectedMakeRequestsErrors", or "expectedMakeBidsErrors") do you think it'd be convenient to add a check? I believe a validation would be helpful to make sure the tests are actually asserting the fields. Something inside adapters/adapterstest/test_json.go that could look like:

func verifyJsonTestFields(spec testSpec, filename string) error {
    emptyBidRequest := openrtb.BidRequest{}

    // Verify bid request is not empty
    if cmp.Equal(spec.BidRequest, emptyBidRequest) {
        return fmt.Errorf("Json field mockBidRequest should not be empty. Test %s\n", filename)
    }

    // Verify HttpCalls is not empty
    if len(spec.HttpCalls) == 0 && len(spec.MakeRequestErrors) == 0 {
        return fmt.Errorf("We expect either httpCalls or expectedMakeRequestsErrors to be empty but not both. Test %s\n", filename)
    }

    return nil
}
adapters/adapterstest/test_json.go

Thoughts?

[mansinahar]

@guscarreon Yes, that's something that I am gonna put as a follow-up PR. Didn't want to overload this further so kept it limited to just modifying the JSON tests themselves but yeah that one is coming soon :)