Tests: Increase coverage for floors validation by osulzhenko · Pull Request #3741 · prebid/prebid-server-java

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


        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

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

        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

        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]


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

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


Bid request without floors?

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

+        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

+        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

+        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

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

    }

-    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

        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

-                ["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

        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

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


Why can't we use getDefaultBidRequest()?

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

+        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

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


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

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


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

+        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

+        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

+
+        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

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!

2 participants

osulzhenko commented Feb 7, 2025 •

edited

Loading