-
Notifications
You must be signed in to change notification settings - Fork 13
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 advertising related player tests #349
Conversation
b97d304
to
2e6fc90
Compare
if (type !== 'progressive') { | ||
await expectEvent(EventType.AdManifestLoaded, 20); | ||
} |
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 experienced longer waiting times with the Android emulator, therefore added this extra expectation to stabilize the test. APPLIES TO THE OTHER OCCURRENCES TOO
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.
LGTM!
EventType.AdFinished, | ||
EventType.AdBreakFinished | ||
), | ||
18 |
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 there a reason for this quite specific 18? Just a though, what if we increase the default timeout to account for Android emulators being a bit slower and set it to 20?
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 would not touch the default timeout as usually 10 seconds is pretty long already.
This is specific for ads on the Android emulator so belongs to the specific test.
Description
These changes add player integration tests to verify advertising-related events are emitted.
Changes
Added advertising-related player integration tests.
Checklist
CHANGELOG
entry - internal changes only, no changelog needed