Skip to content
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

test websocket origin set in worker #9737

Merged
merged 1 commit into from Mar 14, 2016

Conversation

@g-k
Copy link
Contributor

g-k commented Feb 24, 2016

Fixes #9535

This crashes and I'm not sure how to debug it (and whether it's a problem with the browser or more likely a problem with the test):

./mach test-wpt /websockets/opening-handshake/003-sets-origin.worker --log-raw wpt.log                         Running 1 tests in web-platform-tests

  ▶ CRASH [expected OK] /websockets/opening-handshake/003-sets-origin.worker

Ran 1 tests finished in 2.0 seconds.
  • 0 ran as expected. 0 tests skipped.
  • 1 tests crashed unexpectedly

Also, should the test file be in /websockets/ or /websockets/opening-handshake/?

Review on Reviewable

@highfive
Copy link

highfive commented Feb 24, 2016

warning Warning warning

  • This pull request adds a file without the .ini file extension to tests/wpt/metadata. Please consider removing it!
@jdm
Copy link
Member

jdm commented Feb 24, 2016

Try running with --debugger=gdb (or lldb as required) and setting a breakpoint on rust_panic?

@g-k g-k force-pushed the g-k:test-websocket-origin-in-worker branch from 6cac6e8 to 7dccc3a Feb 24, 2016
@g-k
Copy link
Contributor Author

g-k commented Feb 24, 2016

I get timeouts running from a debugger and crashes running it directly. 👻

Is it possible to crash without rust panicking (e.g. from C code) or am I not using the debugger properly?

./mach test-wpt /websockets/opening-handshake/003-sets-origin.worker                                              test-websocket-origin-in-worker 
Running 1 tests in web-platform-tests

  ▶ CRASH [expected OK] /websockets/opening-handshake/003-sets-origin.worker

Ran 1 tests finished in 2.0 seconds.
  • 0 ran as expected. 0 tests skipped.
  • 1 tests crashed unexpectedly

 greg  ~  servo  ./mach test-wpt /websockets/opening-handshake/003-sets-origin.worker --debugger=gdb
Running 1 tests in web-platform-tests

  [0/1] /websockets/opening-handshake/003-sets-origin.worker
Reading symbols from /Users/greg/servo/target/debug/servo...Reading symbols from /Users/greg/servo/target/debug/servo.dSYM/Contents/Resources/DWARF/servo...done.
done.
(gdb) b rust_panic
Breakpoint 1 at 0x102face74
(gdb) r
Starting program: /Users/greg/servo/target/debug/servo --cpu --hard-fail -u Servo/wptrunner http://web-platform.test:8000/websockets/opening-handshake/003-sets-origin.worker --user-stylesheet /Users/greg/servo/resources/ahem.css
Unable to find Mach task port for process-id 19548: (os/kern) failure (0x5).
 (please check gdb is codesigned - see taskgated(8))
  ▶ TIMEOUT [expected OK] /websockets/opening-handshake/003-sets-origin.worker

Ran 1 tests finished in 12.0 seconds.
  • 0 ran as expected. 0 tests skipped.
  • 1 tests timed out unexpectedly

 greg  ~  servo  ./mach test-wpt /websockets/opening-handshake/003-sets-origin.worker --debugger=lldb
Running 1 tests in web-platform-tests

  [0/1] /websockets/opening-handshake/003-sets-origin.worker
(lldb) target create "/Users/greg/servo/target/debug/servo"
Current executable set to '/Users/greg/servo/target/debug/servo' (x86_64).
(lldb) settings set -- target.run-args  "--cpu" "--hard-fail" "-u" "Servo/wptrunner" "http://web-platform.test:8000/websockets/opening-handshake/003-sets-origin.worker" "--user-stylesheet" "/Users/greg/servo/resources/ahem.css"
(lldb) b rust_panic
Breakpoint 1: where = servo`rust_panic, address = 0x0000000102face70
(lldb) r
Process 19654 launched: '/Users/greg/servo/target/debug/servo' (x86_64)
Process 19654 exited with status = 0 (0x00000000) 
  ▶ TIMEOUT [expected OK] /websockets/opening-handshake/003-sets-origin.worker

Ran 1 tests finished in 21.0 seconds.
  • 0 ran as expected. 0 tests skipped.
  • 1 tests timed out unexpectedly

 greg  ~  servo  lldb --version
lldb-340.4.119.1
 greg  ~  servo  gdb --version
GNU gdb (GDB) 7.10.1
...
@Ms2ger
Copy link
Contributor

Ms2ger commented Feb 24, 2016

Not sure what's up with the crash, but you'll need to add done(); at the end of the js file.

ws.onclose = t.step_func(function(e) {
assert_equals(e.wasClean, true);
ws.onclose = t.step_func(function() {assert_unreached()});
setTimeout(t.step_func_done(function() {done();}), 50)

This comment has been minimized.

@Ms2ger

Ms2ger Feb 26, 2016

Contributor

Just put the done() call as the final line of the file; it signals that all the async_tests have been defined, not that they have all finished. That's what the t.done() call here does.

@Ms2ger Ms2ger self-assigned this Feb 26, 2016
@g-k g-k force-pushed the g-k:test-websocket-origin-in-worker branch from 254fd2c to 53a165c Feb 27, 2016
@Ms2ger
Copy link
Contributor

Ms2ger commented Feb 29, 2016

Thanks! Please address these comments and squash your commits into one.

-S-awaiting-review +S-needs-code-changes


Reviewed 1 of 1 files at r4, 2 of 2 files at r5.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


tests/wpt/web-platform-tests/websockets/Send-data.worker.js, line 22 [r5] (raw file):
Please revert this change.


tests/wpt/web-platform-tests/websockets/opening-handshake/003-sets-origin.worker.js, line 12 [r5] (raw file):
Passing t.step_func_done() to setTimeout(), without the callback, will suffice.


Comments from the review on Reviewable.io

refs: #9535
@g-k g-k force-pushed the g-k:test-websocket-origin-in-worker branch from 53a165c to b408f84 Feb 29, 2016
@Ms2ger
Copy link
Contributor

Ms2ger commented Mar 14, 2016

Thanks, and sorry for the delay.

@bors-servo r+


Reviewed 3 of 3 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from the review on Reviewable.io

@bors-servo
Copy link
Contributor

bors-servo commented Mar 14, 2016

📌 Commit b408f84 has been approved by Ms2ger

@bors-servo
Copy link
Contributor

bors-servo commented Mar 14, 2016

Testing commit b408f84 with merge 6895dab...

bors-servo added a commit that referenced this pull request Mar 14, 2016
test websocket origin set in worker

Fixes #9535

This crashes and I'm not sure how to debug it (and whether it's a problem with the browser or more likely a problem with the test):

```
./mach test-wpt /websockets/opening-handshake/003-sets-origin.worker --log-raw wpt.log                         Running 1 tests in web-platform-tests

   CRASH [expected OK] /websockets/opening-handshake/003-sets-origin.worker

Ran 1 tests finished in 2.0 seconds.
  • 0 ran as expected. 0 tests skipped.
  • 1 tests crashed unexpectedly
```

Also, should the test file be in `/websockets/` or `/websockets/opening-handshake/`?

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9737)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 14, 2016

@bors-servo bors-servo merged commit b408f84 into servo:master Mar 14, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@g-k g-k deleted the g-k:test-websocket-origin-in-worker branch Mar 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.