Tests: Increase coverage for floors validation #3741

osulzhenko · 2025-02-07T19:25:01Z

🔧 Type of changes

✨ What's the context?

What's the context for the changes?

🧠 Rationale behind the change

Why did you choose to make these changes? Were there any trade-offs you had to consider?

🔎 New Bid Adapter Checklist

verify email contact works
NO fully dynamic hostnames
geographic host parameters are NOT required
direct use of HTTP is prohibited - implement an existing Bidder interface that will do all the job
if the ORTB is just forwarded to the endpoint, use the generic adapter - define the new adapter as the alias of the generic adapter
cover an adapter configuration with an integration test

🧪 Test plan

How do you know the changes are safe to ship to production?

🏎 Quality check

Are your changes following our code style guidelines?
Are there any breaking changes in your code?
Does your test coverage exceed 90%?
Are there any erroneous console logs, debuggers or leftover code in your changes?

…ors-validation

marki1an · 2025-04-29T09:12:45Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy


        and: "PBS fetch rules from floors provider"
-        cacheFloorsProviderRules(bidRequest, floorsPbsService)
+        cacheFloorsProviderRules(bidRequest, floorsPbsService, BidderName.GENERIC, NONE)


Add import static org.prebid.server.functional.model.bidder.BidderName.GENERIC

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

marki1an · 2025-04-29T10:03:18Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

        and: "Metric alerts.account_config.ACCOUNT.price-floors should be update"
        def metrics = floorsPbsService.sendCollectedMetricsRequest()
        assert metrics[INVALID_CONFIG_METRIC(bidRequest.accountId) as String] == 1
+        assert !metrics[ALERT_GENERAL]


Please add separate "and" section for this assert

marki1an · 2025-04-29T10:04:04Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

        and: "Metric alerts.account_config.ACCOUNT.price-floors should be update"
        def metrics = floorsPbsService.sendCollectedMetricsRequest()
        assert metrics[INVALID_CONFIG_METRIC(bidRequest.accountId) as String] == 1
+        assert !metrics[ALERT_GENERAL]


src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

marki1an · 2025-04-29T10:32:05Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

+        given: "Test start time"
+        def startTime = Instant.now()
+
+        and: "Bid request with floors"


Bid request without floors?

marki1an · 2025-04-29T10:57:45Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

+        given: "Test start time"
+        def startTime = Instant.now()


We have a global startTime variable

…ors-validation

…into functional-tests/floors-validation

marki1an · 2025-04-30T08:19:55Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

+        then: "PBS should log a warning"
+        def message = "Using dynamic data is not allowed"
+        assert response.ext?.warnings[PREBID]*.code == [999]
+        assert response.ext?.warnings[PREBID]*.message == [message]


Can be in line:
assert response.ext?.warnings[PREBID]*.message == ["Using dynamic data is not allowed"]

marki1an · 2025-04-30T08:33:06Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

+        and: "Account with invalid floors config"
+        def account = getAccountWithEnabledFetch(bidRequest.accountId).tap {
+            config.auction.priceFloors.fetch.enabled = true
+        }


Why invalid?
And getAccountWithEnabledFetch is creating to fetch.enabled = true

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

marki1an · 2025-04-30T08:33:57Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

    }

-    def "PBS should emit errors when stored request has more rules than price-floor.max-rules for amp request"() {
+//    @IgnoreRest


Please remove comment

Please check other occurrences

marki1an · 2025-04-30T08:41:48Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy

        assert metrics[FETCH_FAILURE_METRIC] == 1

        and: "PBS log should contain error"
+        def message = "Price floor modelGroup skipRate must be in range(0-100), but was $invalidSkipRate"


Message can be global constant for FetchingSpec

marki1an · 2025-04-30T08:42:42Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy

-                ["Failed to parse price floors from request, with a reason: Price floor modelGroup modelWeight " +
-                         "must be in range(1-100), but was $invalidModelWeight"]
+        and: "Response should contain warning"
+        def message = "Price floor modelGroup modelWeight must be in range(1-100), but was $invalidModelWeight"


Same as previous

marki1an · 2025-04-30T08:43:02Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsFetchingSpec.groovy

        assert metrics[FETCH_FAILURE_METRIC] == 1

        and: "PBS log should contain error"
+        def message = "Price floor rules values can't be null or empty, but were null"


marki1an · 2025-04-30T11:10:25Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

+        def bidRequest = bidRequestWithFloors.tap {
+            ext.prebid.floors = null
+        }


Why can't we use getDefaultBidRequest()?

marki1an · 2025-04-30T11:22:59Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

+        and: "Set bidder response"
+        def bidResponse = BidResponse.getDefaultBidResponse(bidRequest)
+        bidder.setResponse(bidRequest.id, bidResponse)


We don't need here bidder response

marki1an · 2025-04-30T11:25:16Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

+        and: "Set bidder response"
+        def bidResponse = BidResponse.getDefaultBidResponse(bidRequest)
+        bidder.setResponse(bidRequest.id, bidResponse)


marki1an · 2025-04-30T11:25:36Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

+        and: "Set bidder response"
+        def bidResponse = BidResponse.getDefaultBidResponse(bidRequest)
+        bidder.setResponse(bidRequest.id, bidResponse)


marki1an · 2025-04-30T11:25:59Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

+        bidder.setResponse(bidRequest.id, bidResponse)
+
+        and: "PBS fetch rules from floors provider"
+        cacheFloorsProviderRules(bidRequest, floorsPbsService, GENERIC, SUCCESS)


Just cacheFloorsProviderRules(bidRequest)

marki1an · 2025-04-30T11:28:04Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

+        and: "PBS request status shouldn't be in progress"
+        def bidderRequest = bidder.getBidderRequests(bidRequest.id)
+        assert getRequests(bidResponse)[GENERIC.value]?.first?.ext?.prebid?.floors?.fetchStatus == NONE
+        assert bidderRequest.ext.prebid.floors.fetchStatus == [NONE]


Please split these two assertions by and: sections

marki1an · 2025-04-30T11:29:13Z

src/test/groovy/org/prebid/server/functional/tests/pricefloors/PriceFloorsSignalingSpec.groovy

+
+        and: "Account with enabled fetching"
+        def account = getAccountWithEnabledFetch(bidRequest.accountId).tap {
+            config.auction.priceFloors.fetch.enabled = true


getAccountWithEnabledFetch() already setup enabled to true

Increase coverage for floors validation

56c395d

osulzhenko added work in progress Signals not finished work tests Functional or other tests labels Feb 7, 2025

osulzhenko requested a review from marki1an February 7, 2025 19:25

osulzhenko self-assigned this Feb 7, 2025

Increase coverage for floors validation

25e6876

osulzhenko removed the work in progress Signals not finished work label Feb 10, 2025

osulzhenko marked this pull request as ready for review February 10, 2025 12:05

osulzhenko added the blocked label Feb 21, 2025

osulzhenko changed the title ~~Increase coverage for floors validation~~ Tests: Increase coverage for floors validation Feb 28, 2025

osulzhenko added 2 commits April 24, 2025 12:10

Merge branch 'refs/heads/price-floors-logs' into functional-tests/flo…

245aaee

…ors-validation

update tests

0dba5f9

osulzhenko changed the base branch from master to price-floors-logs April 27, 2025 16:16

osulzhenko removed the blocked label Apr 28, 2025

osulzhenko added 3 commits April 28, 2025 11:51

update tests

052a442

update tests

deab6b8

Merge branch 'refs/heads/price-floors-logs' into functional-tests/flo…

f6579b0

…ors-validation

marki1an requested changes Apr 29, 2025

View reviewed changes

osulzhenko added 2 commits April 29, 2025 16:08

Merge branch 'refs/heads/price-floors-logs' into functional-tests/flo…

54963e4

…ors-validation

update tests

331ef99

osulzhenko requested a review from marki1an April 29, 2025 23:36

Merge remote-tracking branch 'refs/remotes/origin/price-floors-logs' …

3ac05a3

…into functional-tests/floors-validation

marki1an requested changes Apr 30, 2025

View reviewed changes

update after review

1925824

osulzhenko requested a review from marki1an April 30, 2025 10:56

marki1an requested changes Apr 30, 2025

View reviewed changes

update after review

03ce834

marki1an approved these changes Apr 30, 2025

View reviewed changes

osulzhenko merged commit 8377468 into price-floors-logs Apr 30, 2025
1 check passed

osulzhenko deleted the functional-tests/floors-validation branch April 30, 2025 12:05

Tests: Increase coverage for floors validation #3741

Tests: Increase coverage for floors validation #3741

Uh oh!

Conversation

osulzhenko commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔧 Type of changes

✨ What's the context?

🧠 Rationale behind the change

🔎 New Bid Adapter Checklist

🧪 Test plan

🏎 Quality check

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

osulzhenko commented Feb 7, 2025 •

edited

Loading