Make more browser tests

[eiriksm]

Hey.

As mentioned in #1376 I started seeing if I could write a more extensive browser test suite. So first things first, we now have coverage info (via that issue).

So next thing I started doing was just copying some tests from the "server side test-suite", and noticed a couple of things. I am going to use test-toJSON as an example, since it is fairly small and easy.

Tests are starting servers

This is an obvious one. And why shouldn't tests start servers, right? The thing is that I can make all assertions in test-toJSON pass, if I only execute the parts that does not open or close the server.

So my question is: Wouldn't the ideal thing here be to share the test cases? Not sure what the ideal model would be, but for example this would work:

'use strict'

var request = require('../index')
  , http = require('http')
  , tape = require('tape')

var s = http.createServer(function (req, resp) {
  resp.statusCode = 200
  resp.end('asdf')
})

tape('setup', function(t) {
  s.listen(6767, function() {
    t.end()
  })
})

require('./common/test-toJSON')(tape, request);

tape('cleanup', function(t) {
  s.close(function() {
    t.end()
  })
})

...or something like that.

The other alternative is to duplicate the tests that are actually relevant for the browser. Which of course is not every test anyway.

Starting servers for the browser.

My next question is. Some of the tests will not work via CORS. For example are you not allowed to read all the same headers when the request is cross domain. Luckily karma has a built in proxy, so you can actually get /basic to go to a server with basic auth (for example). What is our best course of action here? Should we try to reuse the servers in the tests also? Should we add some routing to the one server, and make it serve what we need for the tests we need?

...or should we just be pragmatic and toss in whatever we can make pass into the tests/browser directory?

And final question: one PR per test suite? A PR now and then with a couple of tests suites?

Thoughts?

[nylen]

I had the same question about how we would handle browser tests spawning servers. I definitely like the idea of sharing tests between the browser and server where possible. I'd rather find a better way than splitting tests into separate modules though, it makes the logic really hard to follow.

How difficult do you think it would be to make createServer in test/server.js handle this for us? What I have in mind is basically that code would behave differently in the browser:

• server.createServer() would return some kind of object s with methods that communicate back to some test harness code, using dnode or similar RPC mechanism
• s.on(url) would register a request handler on the server, this would mean we'd need another communication mechanism than closing over variables in the request handler
• s.listen and s.close would communicate back to the test harness the same way and call the callback when the requested operation is complete

To me, that's the ideal way to handle it: the test code for relevant cases is the same, and we call helper methods that do different things whether we're in the browser or Node.js.

If there are issues with this approach that I'm not aware of, then I think we still want to be pragmatic as you said and just get something that works.

---

I'm not sure I understand enough about how karma works to answer your question about CORS and proxies - does karma start a server and serve a test page over that, then we can specify routes on that same server to proxy to other destinations? Sounds like a reasonable way to get it working to me, especially if our createServer helpers could handle that setup and teardown.

---

I don't have a strong preference on PR structure (or a lot of this other stuff really, just trying to figure out a good design which is a long-term benefit if we can go ahead and do it). I definitely support any work towards getting browser tests working and improving coverage :)

[eiriksm]

Yes! I was also thinking about that the createServer method could do this the other day. That would be kind of neat. I think I'll think a little bit about that first (or if someone else wants to jump on it, please go ahead!)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.