Support and test on-demand global mode #1593
Conversation
| @@ -781,7 +781,7 @@ suite('p5.Color', function() { | |||
| }); | |||
| }); | |||
|
|
|||
| suite.only('p5.Color.prototype.toString', function() { | |||
| suite('p5.Color.prototype.toString', function() { | |||
There was a problem hiding this comment.
Looks like this was introduced in #1463. It didn't seem to affect running tests from the command-line, but when run in-browser it'd configure mocha to only run this suite and no others.
There was a problem hiding this comment.
Oh, I was wrong about this not affecting tests from the command-line. It does affect tests from the command-line! In fact, f024aab broke the tests, but they didn't show up in Travis CI because of this line 😞
|
Er... I am really confused by the test failure. As far as I can tell from looking at the source, the test that's failing should have started failing as of f024aab... |
|
Hmm, in fact, that test is failing when I run it in my browser on |
|
Blerg, figured it out--see #1593 (comment). @lmccart any idea what to do about the failing test introduced in f024aab? |
|
Was just about to say it was that commit. I think we can just fix this line in the test definition. |
|
Cool, do you think we should just change it to test if the result is an |
|
oops, yes we can just change that to object. I wonder why it didn't fail when I ran it. sorry about that! |
|
No worries! It didn't fail when you ran it because of the Just committed a fix, so this should be ready for review! |
| @@ -471,6 +471,7 @@ var p5 = function(sketch, node, sync) { | |||
| // assume "global" mode and make everything global (i.e. on the window) | |||
| if (!sketch) { | |||
| this._isGlobal = true; | |||
| p5.instance = this; | |||
There was a problem hiding this comment.
If we want to be really friendly, we could actually add a check here to see if p5.instance is non-null, and log a friendly warning if so. Hopefully that should never happen, but it could potentially cause some hard-to-catch bugs if it ever does.
|
this looks all good, very helpful stuff!! |
This adds support and unit tests for on-demand global mode, fixing #1570.
In particular, it fixes a bug whereby p5 would actually initialize itself twice: once when on-demand global mode was triggered via an explicit
new p5(), and again when the page'sloadevent fired (i.e., "non-on-demand global mode"). This would then cause a bunch of warnings from #1318 to be logged.Now
p5.instanceis always written when_isGlobalis set, and it's checked before initializing non-on-demand global mode.Since there weren't already tests for non-on-demand global mode, I also added some.