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

Feature/clean up player objects in tests #3524

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
@rlchung
Contributor

rlchung commented Aug 12, 2016

Description

Fixed Issue #2895 ; cleaned up player objects in tests.
Added player.dispose() function where necessary.

Specific Changes proposed

N/A

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
  • Reviewed by Two Core Contributors

@rlchung rlchung closed this Aug 12, 2016

@rlchung rlchung reopened this Aug 12, 2016

@BrandonOCasey

This comment has been minimized.

Show comment
Hide comment
@BrandonOCasey

BrandonOCasey Aug 12, 2016

Contributor

From a quick once over in each of the files it looks like you got all the dispose calls that we were missing!
EXCELLENT

Contributor

BrandonOCasey commented Aug 12, 2016

From a quick once over in each of the files it looks like you got all the dispose calls that we were missing!
EXCELLENT

@BrandonOCasey

This comment has been minimized.

Show comment
Hide comment
@BrandonOCasey

BrandonOCasey Aug 12, 2016

Contributor

also LGTM 😄

Contributor

BrandonOCasey commented Aug 12, 2016

also LGTM 😄

@misteroneill

This comment has been minimized.

Show comment
Hide comment
@misteroneill

misteroneill Aug 12, 2016

Member

Nice. LGTM. Thanks @rlchung

Ultimately, we'll want to refactor the tests such that this can be achieved in a afterEach step, but this is perfect for now.

Member

misteroneill commented Aug 12, 2016

Nice. LGTM. Thanks @rlchung

Ultimately, we'll want to refactor the tests such that this can be achieved in a afterEach step, but this is perfect for now.

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Aug 15, 2016

Member

LGTM.
Would you be able to rebase this against master?

Member

gkatsev commented Aug 15, 2016

LGTM.
Would you be able to rebase this against master?

fixes #2894 for relevant tests
fixes #2895 for button.test.js

fixes #2895 for controls.test.js

fixed #2895 for setup.test.js

fixed #2895 for video.test.js

fixes #2895 for audio-tracks.test.js

fixed #2895 for text-tracks.test.js

fixed #2895 for video-tracks.test.js

fixes #2895 cleans up player objects in tests

fixes #2895 revision for trailing space error
@rlchung

This comment has been minimized.

Show comment
Hide comment
@rlchung

rlchung Aug 15, 2016

Contributor

@gkatsev Done!

Contributor

rlchung commented Aug 15, 2016

@gkatsev Done!

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Aug 15, 2016

Member

Awesome, thanks!

Member

gkatsev commented Aug 15, 2016

Awesome, thanks!

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Aug 25, 2016

Member

Oops, forgot the # in the commit. This is closed by af6beb2.

Thanks @rlchung.

Member

gkatsev commented Aug 25, 2016

Oops, forgot the # in the commit. This is closed by af6beb2.

Thanks @rlchung.

@gkatsev gkatsev closed this Aug 25, 2016

@rlchung rlchung deleted the rlchung:feature/clean-up-player-objects-in-tests branch Aug 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment