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

fix(PPDSC-2312): audio player test improvements #373

Merged
merged 5 commits into from
Sep 14, 2022

Conversation

mstuartf
Copy link
Contributor

@mstuartf mstuartf commented Sep 13, 2022

PPDSC-2312

What

  1. Background - why this is needed
    Main issues:
  • Player sometimes still in loading state when snapshot taken.
  • Arbitrary waits.
  • Buffered amount changes, making tests flaky.
  • Animations making screenshots inconsistent.
  • Time elapsed in autoplay tests not consistent.
  1. What did you do
    Fixes (to avoid ignore regions in tests):
  • Make sure all players in a test story are loaded before taking the snapshot using oncanplay event.
  • Prefetch src for all players so that content is fully buffered. Add separate tests for loading / unbuffered state.
  • Disable CSS animations to make screenshots consistent.
  • Remove visual tests for autoplay behaviour as this is not necessary. Also remove duplicate autoplay story.
  • Remove all arbitrary waits and ignore regions in tests.
  1. What does the reviewers should expect
    All AudoPlayer and AudioPlayerComposable visual tests should have no arbitrary waits or ignore regions and should consistently pass.

I have done:

  • Written unit tests against changes
  • Written functional tests against the component and/or NewsKit site
  • Updated relevant documentation

I have tested manually:

  • The feature's functionality is working as expected on Chrome, Firefox, Safari and Edge
  • The screen reader reads and flows through the elements as expected.
  • There are no new errors in the browser console coming from this PR.
  • When visual test is not added, it renders correctly on different browsers and mobile viewports (Safari, Firefox, small mobile viewport, tablet)
  • The Playground feature is working as expected

Before:

After:

Who should review this PR:

How to test:

Main issues:
- Player sometimes still in loading state when snapshot taken.
- Arbitrary waits.
- Buffered amount changes, making tests flaky.
- Animations making screenshots inconsistent.
- Time elapsed in autoplay tests not consistent.

Fixes (to avoid ignore regions in tests):
- Make sure all players in a test story are loaded before taking the snapshot using oncanplay event.
- Prefetch src for all players so that content is fully buffered. Add separate tests for loading / unbuffered state.
- Disable CSS animations to make screenshots consistent.
- Remove visual tests for autoplay behaviour as this is not necessary. Also remove duplicate autoplay story.
- Remove all arbitrary waits and ignore regions in tests.
@newskit-engineering
Copy link
Collaborator

@mutebg mutebg added the ready for review Please assist in getting this reviewed label Sep 13, 2022
@mstuartf mstuartf merged commit 962bdbf into main Sep 14, 2022
@mstuartf mstuartf deleted the fix/PPDSC-2312-audio-player-tests branch September 14, 2022 10:22
Xin00163 pushed a commit that referenced this pull request Oct 17, 2022
* fix(PPDSC-2312): audio player test improvements

Main issues:
- Player sometimes still in loading state when snapshot taken.
- Arbitrary waits.
- Buffered amount changes, making tests flaky.
- Animations making screenshots inconsistent.
- Time elapsed in autoplay tests not consistent.

Fixes (to avoid ignore regions in tests):
- Make sure all players in a test story are loaded before taking the snapshot using oncanplay event.
- Prefetch src for all players so that content is fully buffered. Add separate tests for loading / unbuffered state.
- Disable CSS animations to make screenshots consistent.
- Remove visual tests for autoplay behaviour as this is not necessary. Also remove duplicate autoplay story.
- Remove all arbitrary waits and ignore regions in tests.

* fix(PPDSC-2312): correct visual test check

* fix(PPDSC-2312): fix e2e test errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix This change fixes a bug ready for review Please assist in getting this reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants