Skip to content

Comments

Tests: Fix script src for stack-errors test#1003

Closed
jzaefferer wants to merge 1 commit intomasterfrom
stack-errors-reference
Closed

Tests: Fix script src for stack-errors test#1003
jzaefferer wants to merge 1 commit intomasterfrom
stack-errors-reference

Conversation

@jzaefferer
Copy link
Member

Fixed the bad reference, but the test still seems pretty broken.

  • The globals test reports "Introduced global variable(s): external, chrome, document, test, module, expect, start, ok, notOk, equal, notEqual, propEqual, notPropEqual, deepEqual, notDeepEqual, strictEqual, notStrictEqual, throws, raises, QUnit, polluteGlobal, speechSynthesis, caches, ondeviceorientationabsolute, localStorage, sessionStorage, webkitStorageInfo, indexedDB, webkitIndexedDB, ondeviceorientation, ondevicemotion, crypto, postMessage, blur, focus, [...]" and many more.
  • There's plenty false negatives (I think), like "Uncaught Error: pushFailure() assertion outside test contex"

And apparently the whole test suite isn't run as party of npm test, so the very basic issue of a bad file reference went unnoticed until now. As such, maybe the better solution is to delete it?

@trentmwillis
Copy link
Member

And apparently the whole test suite isn't run as party of npm test, so the very basic issue of a bad file reference went unnoticed until now.

We need a way to automate discovering the files to run in our test suite. Currently there is too much manual intervention (you have to update the Gruntfile every time a name changes or a file is added), which I believe is why this was missed for so long.

Looking at the tests though, were these ever passable? I don't see anything to negate the expected failures. I'm also not sure these test anything not covered by our other tests.

@jzaefferer
Copy link
Member Author

I think the purpose of that suite was to manually check if stack traces are reported properly. But since it was broken for quite a while, I guess no one was doing that.

It would be nice to have some automated tests in the regular test suite, then we could delete this one.

@leobalter leobalter closed this in b2338e9 Jun 16, 2016
@leobalter
Copy link
Member

I think the purpose of that suite was to manually check if stack traces are reported properly.

Yes, and while we don't have it improved, I applied this fix.

merged at b2338e9

@leobalter leobalter deleted the stack-errors-reference branch June 16, 2016 15:18
flore77 pushed a commit to flore77/qunit that referenced this pull request Aug 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants