Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Issue #264 - Don't override QUnit.done #265

Closed
wants to merge 1 commit into from

3 participants

dherman Timo Tijhof James M. Greene
dherman

Rather than overriding QUnit.done, register a callback with it so that
it continues to function normally. Overriding it was causing the
behavior of QUnit.done to differ from what was specified in the API docs
which caused some weird behavior with other plugins like QUnit.Composite.

Fixes #264.

dherman dcherman Don't override QUnit.done - fixes gh-264
Rather than overriding QUnit.done, register a callback with it so that
it continues to function normally.  Overriding it was causing the
behavior of QUnit.done to differ from what was specified in the API docs
which caused some weird behavior with other plugins like QUnit.Composite
7658297
dherman

There's no test suite to run and I'm not sure why QUnit.done was being overriden rather than simply used, so please take this PR with a grain of salt.

Timo Tijhof
Collaborator

@dcherman It was bring overwritten because before 2012, that was the way to register the callbacks in QUnit. See also v1.0.0 history and jquery/qunit@1039a2c.

Assigning to the property actually still works fine (QUnit checks at run-time whether the property is a method from the prototype or an own property and if so, treat the property as a callback instead of looping through the internal list.

Though this works, it only works if either everything uses the new way or everything the old way. If something is using the old way (TestSwarm) and something else (your code) the new way, things go bad.

I'll land this though, it's about time!

Timo Tijhof Krinkle closed this
Timo Tijhof
Collaborator

Landed in fed8b59.

Timo Tijhof
Collaborator

I'm checking for jQuery CLA signing. I should have asked for that before merging, could you sign jQuery's CLA now?

Please be sure to use the same name and email address as for committing with Git: http://contribute.jquery.org/CLA/

dherman

I signed it a while ago for some minor contributions to jQuery Core so I should be on there - happy to do so again if you need me to though

Timo Tijhof
Collaborator

@dcherman Indeed you are (I see you now), my mistake. I must've missed it earlier (I think I was searching for "dchermen" instead of D C Herman). Thanks again!

James M. Greene
Collaborator

:+1:

dherman dcherman deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 21, 2013
  1. dherman

    Don't override QUnit.done - fixes gh-264

    dcherman authored
    Rather than overriding QUnit.done, register a callback with it so that
    it continues to function normally.  Overriding it was causing the
    behavior of QUnit.done to differ from what was specified in the API docs
    which caused some weird behavior with other plugins like QUnit.Composite
This page is out of date. Refresh to see the latest.
Showing with 2 additions and 2 deletions.
  1. +2 −2 js/inject.js
4 js/inject.js
View
@@ -223,13 +223,13 @@
return typeof QUnit !== 'undefined';
},
install: function () {
- QUnit.done = function ( results ) {
+ QUnit.done(function ( results ) {
submit({
fail: results.failed,
error: 0,
total: results.total
});
- };
+ });
QUnit.log = window.TestSwarm.heartbeat;
window.TestSwarm.heartbeat();
Something went wrong with that request. Please try again.