-
Notifications
You must be signed in to change notification settings - Fork 190
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
Refactoring functional tests #3657
base: master
Are you sure you want to change the base?
Conversation
6ad3b09
to
26f38ea
Compare
26f38ea
to
c6ddf01
Compare
# Conflicts: # src/test/groovy/org/prebid/server/functional/tests/AmpFpdSpec.groovy
review is postponed |
# Conflicts: # src/test/groovy/org/prebid/server/functional/tests/module/AbTestingModuleSpec.groovy # src/test/groovy/org/prebid/server/functional/tests/module/GeneralModuleSpec.groovy # src/test/groovy/org/prebid/server/functional/tests/module/analyticstag/AnalyticsTagsModuleSpec.groovy # src/test/groovy/org/prebid/server/functional/tests/module/ortb2blocking/Ortb2BlockingSpec.groovy # src/test/groovy/org/prebid/server/functional/tests/module/pbrequestcorrection/PbRequestCorrectionSpec.groovy # src/test/groovy/org/prebid/server/functional/tests/module/richmedia/RichMediaFilterSpec.groovy
# Conflicts: # src/test/groovy/org/prebid/server/functional/tests/privacy/ActivityTraceLogSpec.groovy
@@ -7,15 +7,15 @@ import org.prebid.server.functional.util.PBSUtils | |||
class UserExtData { | |||
|
|||
List<String> keywords | |||
String buyeruid | |||
List<String> buyeruids | |||
String buyerUid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it was changed in tech spec?
@@ -15,12 +14,12 @@ import static org.prebid.server.functional.model.request.auction.FetchStatus.SUC | |||
class AnalyticResult { | |||
|
|||
String name | |||
FetchStatus status | |||
String status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not an enam?
@@ -42,6 +42,12 @@ abstract class BaseSpec extends Specification implements ObjectMapperWrapper { | |||
private static final String DEFAULT_CACHE_DIRECTORY = "/app/prebid-server/data" | |||
protected static final Map<String, String> GENERIC_ALIAS_CONFIG = ["adapters.generic.aliases.alias.enabled" : "true", | |||
"adapters.generic.aliases.alias.endpoint": "$networkServiceContainer.rootUri/auction".toString()] | |||
protected static final Map<String, String> EMPTY_CACHE_TTL_CONFIG = ["cache.default-ttl-seconds.banner": "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formating it please
values() | ||
.collect { [stage: it, matchLength: moduleHook.code.indexOf(it.value) >= 0 ? it.value.length() : -1] } | ||
.findAll { it.matchLength > 0 } | ||
.max { it.matchLength } | ||
?.stage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
values().collect { [stage: it, matchLength: moduleHook.code.indexOf(it.value) >= 0 ? it.value.length() : -1] }
.findAll { it.matchLength > 0 }
.max { it.matchLength }
?.stage
@@ -97,7 +97,8 @@ class CacheSpec extends BaseSpec { | |||
def "PBS should cache bids without api-key header when targeting is specified and api-key-secured disabled"() { | |||
given: "Pbs config with disabled api-key-secured and pbc.api.key" | |||
def apiKey = PBSUtils.randomString | |||
def pbsService = pbsServiceFactory.getService(['pbc.api.key': apiKey, 'cache.api-key-secured': 'false']) | |||
def pbsConfig = ['pbc.api.key': apiKey, 'cache.api-key-secured': 'false'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
['pbc.api.key': apiKey,
'cache.api-key-secured': 'false']
Same for other
] | ||
|
||
private static final PrebidServerService storedRequestIdGenEnabledService = pbsServiceFactory.getService(STORED_REQUEST_ID_GEN_ENABLED_CONFIG) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove empty line
ORTB_ADAPTER_CONFIG + | ||
MODIFYING_VAST_CONFIG + | ||
EXTERNAL_MODULES_CONFIG | ||
) | ||
} | ||
|
||
def cleanupSpec() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about cleanup pbsServiceWithMultipleModules
container?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a base module, meaning it will clean up after the package, and with cleanupSpec, it will drop and restart the container every time. And this is not efficient.
🔧 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
🧪 Test plan
How do you know the changes are safe to ship to production?
🏎 Quality check