Skip to content
This repository was archived by the owner on Jul 5, 2021. It is now read-only.

Conversation

@lpww
Copy link

@lpww lpww commented Feb 7, 2019

I've added a custom TestMediaControlsHook component here that is based on TestHook. It renders the <audio> player and passes the ref through to the callback. It seems to work quite well, although I'm open to other methods if anyone has a suggestion. The same technique can probably be used for the fullScreen hook, which also accepts a reference.

Methods tested:

  • play
  • pause
  • setVolume
  • mute
  • unmute
  • seeks (partially tested - the events are not being triggered by jsdom I think)
  • stop (blocked - uses seek)
  • restart (blocked - uses seek)

@cianfoley-nearform
Copy link
Collaborator

@lpww I investigated the seek and found the events are firing and state being set, the test must be occurring asynchronously before the state is actually set

I put in this and the test passes for example, so maybe there's a way of capturing when the seek is done?

timeout(() => expect(currentTime).toBe(2), 1000)

@lpww
Copy link
Author

lpww commented Feb 7, 2019

@cianfoley-nearform You are right - seek is an async operation. I had to listen for the "seeked" event and assert in the handler. I'm having trouble now though because the cleanup function removes elements in the order they are created. This means that my ref is removed before the hook component, so the cleanup removeEventListener's all fail. I'm still looking into that

@lpww
Copy link
Author

lpww commented Feb 7, 2019

I had to work around the issue by rendering the audio element as a child of the hook element (so it is removed after). Everything is working now and ready for review!

Copy link
Contributor

@jackdclark jackdclark left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@jackdclark jackdclark merged commit fec3982 into master Feb 8, 2019
@jackdclark jackdclark deleted the test-media branch February 8, 2019 09:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants