-
Notifications
You must be signed in to change notification settings - Fork 331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix spurious test failure with "creates new identifiers" [OSF-6970] #6655
Fix spurious test failure with "creates new identifiers" [OSF-6970] #6655
Conversation
From @brianjgeiger in Jira:
|
I find http://stackoverflow.com/questions/38525336/typeerror-fake-xhr-onreadystatechange-undefined-is-not-a-constructor and note that this is a fake XHR onreadystatechange handler. Is |
OSF-7002 was closed by turning relevant tests off in #6463. Here's @brianjgeiger's notes from 7002:
|
…ility" This reverts commit 2e3e333.
I don't understand why this shows up under the Varnish tests. What does Varnish have to do with this? |
The call appears to be:
But why? |
That's an old test run, from three months ago. But the first test run on this PR just completed, and it still runs karma under |
|
Constrained build succeeded in under five minutes. Probably even quicker now that the Travis cache is primed. |
Bah. But I can't restart Travis builds without closing and reopening the PR. :-/ |
Hmm ... did they close that loophole? 😞 |
Build restarted. 👍 |
The full run does fail still, under "create new identifiers" as well as three others. 👍 |
The test suite creates a stub server using sinon. When calling |
I moved the JS tests to the varnish branch because that was the fastest to run. If it had to be restarted, I wanted it to be a five minute wait, not a 35 minute wait. |
Yeah, I figured. It was just confusing since the test suite was still called just "varnish". PR to clean that up a bit in #6656. |
Alright, so what's this about a sinon race condition? |
http://stackoverflow.com/questions/11304225/sinon-js-fake-server-autorespond-usage |
Looking at those docs, both |
We specify
|
1
2
3
4
5
6
7
8
9
10
|
There are ten stack traces, but only seven failures. All the stack traces are the same. |
Since this bug is intermittent, I'm going to restart the build. If we don't get the "create new identifiers" fail it will be interesting to compare the result. |
New plan: try to turn on Sentry reporting. |
I've created a personal project in Sentry and have set |
The reason to use Sentry is because I'm basically rebuilding Sentry with the stacktrace thing. Really, I want to inspect frame context, not just see the stacktrace. |
I'm getting messages logged, but I want stack traces from exceptions. |
Let's see if this dog can hunt ... |
Blech. Looks like it's still a static trace, though. 😕 Is frame context available with Sentry under JavaScript? |
Maybe with a newer JavaScript client? If it's not available at all then I guess Sentry is a bit of a dead-end. |
I read through the Raven docs, upgraded the client, switched to using |
I've reported "Frame context not included." |
Okay! So what can we learn from what we know so far? And can we log at least a variable or two of extra context easily with Raven? |
Gosh this feels dense. I can't get an interactive debugger where I need it? |
https://gitter.im/karma-runner/karma?at=585bc657058ca967377b74c4 |
Back to karma-runner/karma#553 (comment). |
For the record, my contract with COS is up and we didn't renew for 2017, so this is ready for someone else to pick up! :-) |
Closing, as this has become stale. |
Purpose
Debug and fix a spurious test case failure with 37ae2bb under
TEST_BUILD="varnish"
, wherenodeControl > ViewModels > ProjectViewModel > create new identifiers
dies with aTypeError
insidecontribAdder
:Changes
n/a
Side effects
n/a
Ticket
https://openscience.atlassian.net/browse/OSF-6970