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

Add PG tests infrastructure #1581

Merged
merged 11 commits into from
Feb 10, 2022
Merged

Add PG tests infrastructure #1581

merged 11 commits into from
Feb 10, 2022

Conversation

hupaloo
Copy link
Contributor

@hupaloo hupaloo commented Nov 16, 2021

Infrastructure before adding tests on PBS PG functionality

@hupaloo hupaloo force-pushed the add-pg-tests-infrastructure branch 2 times, most recently from cdc197d to 2ec78aa Compare November 16, 2021 13:07
@Net-burst Net-burst added the tests Functional or other tests label Nov 18, 2021
static void waitUntil(Closure closure, long timeout = 1000, long pollInterval = 100) {
with().pollDelay(0, MILLISECONDS)
.pollInterval(pollInterval, MILLISECONDS)
.await()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need the await() method here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That method don't do anything and just return this, but it is used to enhance the readability of test conditions

@@ -32,4 +34,12 @@ class PBSUtils {
static String getRandomString(int stringLength = 20) {
RandomStringUtils.randomAlphanumeric(stringLength)
}

static void waitUntil(Closure closure, long timeout = 1000, long pollInterval = 100) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the functionality of the checkRequestCount method is duplicated, so you can remove it. Or use it instead of the given method

reset(USER_DETAILS_ENDPOINT_PATH)
}

void setResponse() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add Override annotation where necessary for better readability

}

void setUserDataResponse(UserDetailsResponse userDataResponse, HttpStatusCode httpStatusCode = OK_200) {
resetUserDetailsEndpoint()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you using reset here? There were no expectations set for this endpoint before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is an infrastructure PR, expectations are set in the test specs from other PG PRs (e.g. in some test methods from UserDetailsSpec, #1590)

mhupalo and others added 7 commits February 7, 2022 13:25
* Add PG tests for Alert, Currency, Plans, Register services

* Add currency conversion rates mock; fix CurrencySpec

* Moved setting bid request and response to the setup()

* Use soft assert where needed

Co-authored-by: mhupalo <mhupalo@rubiconproject.com>
* Add PG tests to cover auction and tokens

* Removed setting bid response where it doesn't matter

* Use soft assert where needed; parametrization style updated

Co-authored-by: mhupalo <mhupalo@rubiconproject.com>
* Add PG bidder request, bid response, user details specs

* Resolve PR remarks

* Use soft assert where needed

Co-authored-by: mhupalo <mhupalo@rubiconproject.com>
* Add PG report, targeting specs

* Resolve PR remarks

* Add test case to cover domain targeting bug

* Add test cases to cover scalar and array inputs by site/user first party data, bidder params

* Add test case to cover NPE is not thrown by site/user first party data

Co-authored-by: mhupalo <mhupalo@rubiconproject.com>
Co-authored-by: rpanchyk <rpanchyk@rubiconproject.com>
@Net-burst Net-burst force-pushed the add-pg-tests-infrastructure branch from 6e0885c to 054c4d9 Compare February 7, 2022 11:25
@Net-burst Net-burst force-pushed the add-pg-tests-infrastructure branch from b044b51 to c39af80 Compare February 9, 2022 10:08
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

@Net-burst Net-burst merged commit ce49335 into master Feb 10, 2022
@Net-burst Net-burst deleted the add-pg-tests-infrastructure branch February 10, 2022 16:21
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.

3 participants