Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests: Increase test coverage for cookie_sync #2570

Merged

Conversation

marki1an
Copy link
Collaborator

No description provided.

@marki1an marki1an added tests Functional or other tests work in progress Signals not finished work labels Aug 10, 2023
@marki1an marki1an removed the work in progress Signals not finished work label Sep 7, 2023
osulzhenko
osulzhenko previously approved these changes Sep 7, 2023
@@ -5,7 +5,7 @@ import net.minidev.json.annotate.JsonIgnore

enum BidderName {

ALIAS, GENERIC, RUBICON, APPNEXUS, BOGUS, OPENX
ALIAS, GENERIC, RUBICON, APPNEXUS, BOGUS, OPENX, ACEEX, ACUITYADS, GRID, AAX, ADKERNEL, MEDIANET
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rearrange the list and put BOGUS somewhere at the beginning.

@@ -43,9 +41,9 @@ class VendorListResponse {
Boolean usesNonCookieAccess
Boolean deviceStorageDisclosureUrl

static Vendor getDefaultVendor() {
static Vendor getDefaultVendor(Integer id) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

id can be null? If not, please use int to signify this.


List<BidderName> bidders
// Here we use wildcard for different compatibility
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what you meant by this comment.

@@ -141,6 +141,14 @@ class PrebidServerService implements ObjectMapperWrapper {
response.as(CookieSyncResponse)
}

@Step("[POST] /cookie_sync with uids cookie")
CookieSyncResponse sendCookieSyncRequest(CookieSyncRequest request, String uidsCookie) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the point of duplicating the method, but using a String instead of POJO?


def cookies = [:]
Map<String, String> header = null,
String uidsAudit = null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be here at all.

Copy link
Collaborator

@osulzhenko osulzhenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@Net-burst Net-burst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SerhiiNahornyi SerhiiNahornyi merged commit 71afb69 into master Jan 3, 2024
2 checks passed
@SerhiiNahornyi SerhiiNahornyi deleted the increase-tests-coverage-for-cookie-sync-endpoint branch January 3, 2024 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Functional or other tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants