#305 ✓ checked-in
Steven W

Plugin: subtitle - missing unit tests plugin

Reported by Steven W | February 2nd, 2011 @ 01:25 PM | in 0.4

The subtitle and lowerthird plugins don't have unit tests accompanying them. Along with #300, this would enable the testing of all popcorn functionality in one fell swoop.

Comments and changes to this ticket

  • Steven W

    Steven W February 8th, 2011 @ 03:02 PM

    • Milestone set to 0.4
    • Milestone order changed from “24” to “0”
  • Scott Downe

    Scott Downe March 1st, 2011 @ 02:44 PM

    • State changed from “new” to “assigned”
    • Milestone changed from 0.4 to 0.5
    • Assigned user set to “Scott Downe”
    • Milestone order changed from “2” to “0”

    Googlenews is also missing unit tests.

  • Scott Downe

    Scott Downe March 3rd, 2011 @ 10:26 AM

    I think I will make this subtitle only, seeing how we have a separate ticket for lowerthird and googlenews at #357 and #356.

  • Scott Downe

    Scott Downe March 3rd, 2011 @ 10:27 AM

    • Title changed from “Create unit tests for subtitle, lowerthird plugins” to “Plugin: subtitle - missing unit tests plugin”
  • danman
  • Scott Downe

    Scott Downe March 7th, 2011 @ 04:43 PM

    I see in one of your commit messages "Made index.html able to automate all unit tests. Also fixed an error in the wiki unit test reported by firebug " if so, awesome.

  • annasob

    annasob March 8th, 2011 @ 03:17 PM

    Scott is this good? Can i stage these tests?

  • annasob

    annasob March 8th, 2011 @ 03:19 PM

    Dan, topically each ticket need its own branch. This time we will cherry-pick this one commit but next time please make a new branch. This new branch should be clean, meaning it should have only staged commits and the functionality you are adding for this ticket only.

  • Scott Downe

    Scott Downe March 8th, 2011 @ 03:52 PM

    I think we should take the tests in his commit, and put them into 0.4 if at all possible.

  • annasob

    annasob March 8th, 2011 @ 03:54 PM

    • State changed from “assigned” to “review-needs-work”
    • Milestone changed from 0.5 to 0.4
    • Assigned user changed from “Scott Downe” to “danman”
    • Milestone order changed from “21” to “0”

    Im failing this. I thought i can cherry-pick but the branch itself is a mess. Dan please redo the branch and make a new branch. I dont see you on irc so let me give you some commands:

    git checkout master
    git pull git://github.com/annasob/popcorn-js.git 0.4
    git branch 305
    git checkout 305
    

    the above will give you a fresh branch. You can add the tests manually or cherry-pick them

    git cherry-pick 0bc6fadf909a
    git cherry-pick 56dc8f9568 (may cause one merge error to fix: modify file, git add it, git commit)
    

    That being said I believe the intervals are causing the tests to break in FF Beta. Please use exec() for tests as done in #374

  • danman

    danman March 8th, 2011 @ 08:21 PM

    • State changed from “review-needs-work” to “peer-review-requested”

    https://github.com/DanVentura/popcorn-js/commit/8a887407f773a151867...

    Done. Branch is 305. I promise all the branching problems I've caused you today wont happen again :) Thanks for the help

  • Scott Downe

    Scott Downe March 9th, 2011 @ 12:20 PM

    • State changed from “peer-review-requested” to “super-review-requested”

    Read my comment on #321 for more info, but I think this is a good step forward. I recommend staging this and #321 in one shot, as they are on the same branch (I believe).

  • danman

    danman March 9th, 2011 @ 01:55 PM

    https://github.com/DanVentura/popcorn-js/tree/305b/plugins

    I agree. Here is branch 305b: merged revised subtitle tests from t321b with index.html from 305

  • Rick

    Rick March 9th, 2011 @ 07:02 PM

    • State changed from “super-review-requested” to “review-needs-work”

    The unit test html file is erroneously duplicating an request to the google api...

    This needs to be removed:

      <script src="http://google.com/jsapi"></script>
      <script>google.load("language", "1");</script>
    

    The call to...

    
    Popcorn.getScript( "http://www.google.com/jsapi", function() {
      google.load("language", "1", {callback: function() {scriptLoaded = true;}});
    });
    

    Is sufficient and correct.

  • annasob

    annasob March 9th, 2011 @ 07:38 PM

    • State changed from “review-needs-work” to “staged”

    This is fine. Rick tested the unit tests on FF Beta 13 and they worked. I NINJAd this and deleted the duplicate code before staging.
    The tests pass on Vista Chrome and Firefox 3.6

    Staged in annasob/popcorn-js commit

  • annasob

    annasob March 21st, 2011 @ 02:47 PM

    • State changed from “staged” to “checked-in”
  • annasob

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Popcorn.js is an HTML5 video framework that lets you bring elements of the web into your videos.

Popcorn.js is a project of Web Made Movies, Mozilla's Open Video Lab.

Shared Ticket Bins

Tags

Referenced by

Pages