Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Sandboxed Core Unit Tests #268

Open
wants to merge 4 commits into
from

Conversation

Projects
None yet
2 participants
Owner

cadecairos commented Jan 16, 2013

List of changes:

  1. Updated test runner, so it can run within an instance of itself, and results are funneled to the top.
  2. Every core test has been split into a file and put into sub directories under /test/core
  3. /test/core/index.html is the core test runner
  4. Various fixes to unit tests so they complete in all browsers (nothing changing what they're testing, just fixes to allow for quirks in media loading and seeking)
  5. HTML5 fixes <meta charset="UTF-8" >
  6. CSS3 fixes - dropped use of -moz-border-radius

We'll need to review this one carefully for copy-pasta errors, though I did my best to avoid making them/fixing them. The tests all run far better across all browsers now, and should be easier to debug/create in the future.

Tested working in Firefox 18 (Mac), Chrome 24(Mac), Safari 6(Mac(duh)), Opera 12.12(Mac) and IE9(Win 7) All passing 100% every time! weeee

Note: IE9 may want to use compatibility view, you'll need to turn it off - this helped me: http://help.yahoo.com/kb/index?page=content&y=PROD_FIN&locale=en_AU&id=SLN5276

Have fun!

@mjschranz mjschranz commented on an outdated diff Jan 16, 2013

test/core/audio/audio-basic-raf.html
@@ -0,0 +1,98 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>Popcorn Test Suite</title>
+ <meta charset="UTF-8">
+<link rel="stylesheet" href="../../qunit/qunit.css" type="text/css" media="screen">
@mjschranz

mjschranz Jan 16, 2013

Contributor

SPACING

@mjschranz mjschranz commented on an outdated diff Jan 16, 2013

test/core/audio/audio-basic.html
@@ -0,0 +1,95 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>Popcorn Test Suite</title>
+ <meta charset="UTF-8">
+<link rel="stylesheet" href="../../qunit/qunit.css" type="text/css" media="screen">
@mjschranz

mjschranz Jan 16, 2013

Contributor

SPACING

@mjschranz mjschranz commented on an outdated diff Jan 16, 2013

test/core/core/core-bogus-selector.html
@@ -0,0 +1,65 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>Popcorn Test Suite</title>
+ <meta charset="UTF-8">
+<link rel="stylesheet" href="../../qunit/qunit.css" type="text/css" media="screen">
@mjschranz

mjschranz Jan 16, 2013

Contributor

You get the picture.

@mjschranz mjschranz commented on the diff Jan 17, 2013

test/qunit/qunit.css
@@ -39,7 +39,6 @@
font-weight: normal;
border-radius: 5px 5px 0 0;
- -moz-border-radius: 5px 5px 0 0;
@mjschranz

mjschranz Jan 17, 2013

Contributor

I'm not entirely sure why this needed to be removed but in my mind if you are going to you might as well remove the webkit prefixed ones as well.

@cadecairos

cadecairos Jan 17, 2013

Owner

removed because of errors in the FX console. no errors associated with the other one, so I won't touch it.

@mjschranz mjschranz commented on the diff Jan 17, 2013

test/qunit/qunit.css
@@ -114,7 +113,6 @@
background-color: #fff;
border-radius: 5px;
- -moz-border-radius: 5px;
@mjschranz

mjschranz Jan 17, 2013

Contributor

Same as above.

@mjschranz mjschranz commented on the diff Jan 17, 2013

test/qunit/qunit.css
@@ -191,7 +189,6 @@
#qunit-tests > li:last-child {
border-radius: 0 0 5px 5px;
- -moz-border-radius: 0 0 5px 5px;
@mjschranz

mjschranz Jan 17, 2013

Contributor

Same as above.

@mjschranz mjschranz commented on the diff Jan 17, 2013

test/core/core/core-destroy-events-trackevents.html
+ p.cue( 1, Popcorn.nop );
+
+ p.destroyable({
+ start: 1,
+ end: 2,
+ text: "hi"
+ });
+
+ p.destroy();
+
+ equal( Popcorn.instances.length, 0, "The instance was removed from the Popcorn.instances array." );
+
+ equal( p.data.trackEvents.byStart.length, 0, "Zero trackEvents.byStart after destroy" );
+
+ equal( p.data.trackEvents.byEnd.length, 0, "Zero trackEvents.byEnd after destroy" );
+
@mjschranz

mjschranz Jan 17, 2013

Contributor

Is there a reason why https://github.com/mozilla/popcorn-js/blob/master/test/popcorn.unit.js#L731 was removed from this test group?

@cadecairos

cadecairos Jan 17, 2013

Owner

no need to do that kind of clean up if everything's sandboxed.

@mjschranz mjschranz commented on the diff Jan 17, 2013

test/core/core/core-gettrackevents.html
+ <script src="../../../popcorn.js"></script>
+ <script src="../../popcorn.inject.js"></script>
+ <script>
+ test( "Popcorn.getTrackEvents", 7, function() {
+
+ var popcorn = Popcorn( "#video" );
+
+ equal( typeof Popcorn.getTrackEvents, "function", "Popcorn.getTrackEvents() is a provided static function" );
+
+ equal( typeof Popcorn.getTrackEvents.ref, "function", "Popcorn.getTrackEvents.ref() is a private use static function" );
+
+ equal( typeof Popcorn.getTrackEvents( popcorn ), "object", "Popcorn.getTrackEvents() returns an object" );
+
+ equal( Popcorn.getTrackEvents( popcorn ).length, 0, "Popcorn.getTrackEvents() currently has no trackEvents" );
+
+ popcorn.cue( 1, function(){ });
@mjschranz

mjschranz Jan 17, 2013

Contributor

Might as well fix the spacing while your here.

@cadecairos

cadecairos Jan 17, 2013

Owner

not gonna do fixes on spacing that were already present in the test file. we can file tickets to do that.

@mjschranz mjschranz commented on the diff Jan 17, 2013

test/core/core/core-no-conflict.html
+ <script src="../../qunit/qunit.js"></script>
+ <script src="../../popcorn.unit.setup.js"></script>
+ <script src="../../../popcorn.js"></script>
+ <script src="../../popcorn.inject.js"></script>
+ <script>
+ test( "noConflict", 6, function() {
+
+ ok( Popcorn.noConflict, "Popcorn.noConflict exists" );
+ equal( typeof Popcorn.noConflict, "function", "Popcorn.noConflict is a function" );
+
+ var $$ = Popcorn;
+
+ deepEqual( Popcorn, Popcorn.noConflict(), "noConflict returned the Popcorn object" );
+ deepEqual( Popcorn, $$, "Make sure Popcorn wasn't touched." );
+
+
@mjschranz

mjschranz Jan 17, 2013

Contributor

Might as well remove this.

@mjschranz mjschranz commented on the diff Jan 17, 2013

test/core/core/core-popcorn-static.html
+ <script src="../../jquery.js"></script>
+ <script src="../../qunit/qunit.js"></script>
+ <script src="../../../popcorn.js"></script>
+ <script src="../../popcorn.inject.js"></script>
+ <script>
+ module( "Popcorn Static" );
+
+ test( "Popcorn.[addTrackEvent | removeTrackEvent].ref()", 2, function() {
+
+ var popped = Popcorn( "#video" );
+
+ // Calling exec() will create tracks and added them to the
+ // trackreference internally
+ popped.cue( 1, function() { /* ... */ });
+ popped.cue( 3, function() { /* ... */ });
+ popped.cue( 5, function() { /* ... */ });
@mjschranz

mjschranz Jan 17, 2013

Contributor

Spacing.

@mjschranz mjschranz commented on the diff Jan 17, 2013

test/core/core/core-remove-trackevent.html
+ // This condition should NEVER evaluate to true
+ if ( end._id === aId ) {
+ ok( false, "No byEnd track events with " + aId + " should exist" );
+ }
+ // This condition should ALWAYS evaluate to true
+ if ( end._id === bId ) {
+ ok( true, "Only byEnd track events with " + bId + " should exist" );
+ }
+ }
+ });
+
+ // after the removal, byStart's first element is c
+ // console.log( byStart );
+
+ // after the removal, byEnd's first element should be d (if c, then broken)
+ // console.log( byEnd );
@mjschranz

mjschranz Jan 17, 2013

Contributor

Mind removing these commented lines/console.logs? I don't see the value of them being there if in order to know if the test is broken you have to come in and uncomment the logs.

@cadecairos

cadecairos Jan 17, 2013

Owner

same as above

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