-
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
Functional tests for module execution host config check #3589
Functional tests for module execution host config check #3589
Conversation
|
||
and: "hook call metrics should be updated" | ||
and: "Response-correction module call metrics should be updated" |
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.
"PB_RICHMEDIA_FILTER module call metrics should be updated"
and: "hook call metrics shouldn't be updated" | ||
assert !metrics[CALL_METRIC.formatted(PB_RICHMEDIA_FILTER.code, ALL_PROCESSED_BID_RESPONSES.metricValue, PB_RICHMEDIA_FILTER_ALL_PROCESSED_RESPONSES.code)] | ||
assert !metrics[CALL_METRIC.formatted(PB_RESPONSE_CORRECTION.code, ALL_PROCESSED_BID_RESPONSES.metricValue, RESPONSE_CORRECTION_ALL_PROCESSED_RESPONSES.code)] | ||
and: "Response-correction module call metrics should be updated" |
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.
"PB_RICHMEDIA_FILTERmodule call metrics should be updated"
and: "Save account without modules config" | ||
def accountConfig = new AccountConfig(hooks: new AccountHooksConfiguration(modules: null)) | ||
def account = new Account(uuid: bidRequest.getAccountId(), config: accountConfig) | ||
accountDao.save(account) |
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.
I hope we can delete the account config with empty modules.
and: "Save account without modules config" | ||
def accountConfig = new AccountConfig(hooks: new AccountHooksConfiguration(modules: null)) | ||
def account = new Account(uuid: bidRequest.getAccountId(), config: accountConfig) | ||
accountDao.save(account) |
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.
Same here we can delete it
pbsServiceFactory.removeContainer(pbsConfig) | ||
} | ||
|
||
def "PBS should call all modules without account config when modules disabled in module-execution host 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.
PBS shouldn't allow call module when module disabled in hooks.admin.module-execution
|
||
@ToString(includeNames = true, ignoreNulls = true) | ||
@JsonNaming(PropertyNamingStrategies.LowerCaseStrategy) | ||
class TraceOutcome { | ||
|
||
Stage entity | ||
String entity |
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 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.
Because BIDDER_REQUEST, RAW_BIDDER_RESPONSE, and PROCESSED_BIDDER_RESPONSE from beginning had bidder as entity and stage as a separate parameter, we didn’t have any tests for it previously, so it was implemented as a Stage. However, this is not accurate. You can check it here:
#1345
🔧 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