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

test: cleanup test warnings #4752

Merged
merged 3 commits into from Nov 16, 2017

Conversation

Projects
None yet
3 participants
@BrandonOCasey
Contributor

BrandonOCasey commented Nov 16, 2017

Description

Currently we get logs in the tests due to a recent change. This PR fixes that. It also fixes a case were we would log that the dom element isn't in the dom, when it really is:

Here is an example (taken from the test that showed me it)

const fixture = document.getElementById('qunit-fixture');

fixture.innerHTML += '<video id="test_vid_id"></video>';

const tag = document.getElementById('test_vid_id');

const player = videojs(tag)

// this would log a warning about tag not being in the dom. Technically its not because
// of the changes we made to it. I would even say that this is a bad way to get the player again.
// but I think that warning that the players tag is not in the dom on the first creation is enough, and // warning that it isn't in the dom because the tag has been changed is a bit weird.

// returns the same player as above.
playerAgain(tag);
Show outdated Hide outdated src/js/video.js
Show outdated Hide outdated src/js/video.js
Show outdated Hide outdated test/unit/player.test.js

@gkatsev gkatsev added the confirmed label Nov 16, 2017

@BrandonOCasey

This comment has been minimized.

Show comment
Hide comment
@BrandonOCasey

BrandonOCasey Nov 16, 2017

Contributor

See #4755 for the other PR

Contributor

BrandonOCasey commented Nov 16, 2017

See #4755 for the other PR

@gkatsev gkatsev merged commit 3aae4b2 into master Nov 16, 2017

4 checks passed

continuous-integration/codeship Build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@gkatsev gkatsev deleted the test/cleanup-logs branch Nov 16, 2017

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