Capture server test helper, uhm, help?

[Christian Johansen]

August,

I finalized buster-server-cli. It works when I try it manually,
but I have some tests that verify the behavior of capturing
browsers that are not passing. I'm using the capture server test
helper, and probably just missed some simple thing.

Here's what I did:

    helper.get("/", function (res, body) {
        helper.captureSlave("Mozilla/5.0 (X11; Linux x86_64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1", function () {
            // Logging captureServer.slaves() here yields []

            helper.get("/", done(function (res, body) {
                assert.match(body, "<li class=\"firefox linux\">");
                assert.match(body, "<h3>Firefox 4.0.1 | Linux</h3>");
            }));
        });
    });

This fails because there apparently are no slaves. As you can see
above, logging the slaves after capturing also shows no slaves.
Weirdly, adding a timeout (I tried 100ms) does not
work ("weirdly" because, like I said, upon manual testing it does
actually work).

I poked around a little, and verified that the capture server
does indeed capture the client, but I stumbled on some behavior that
wasn't clear to me. Line #211 in capture-server/lib/server.js I see:

    _onSlaveCaptured: function (slave) {
        this._pubsubClient.emit("slave:captured", {
            slave: slave.serialize(),
            slaves: this._slaves().map(function (s) { return s.serialize() })
        });
    },

The captured event is emitted, and the slave object is the right
object. However, this._slaves() still returns an empty array here.
So I was wondering - when is the new slave added to the array, and what's
wrong with my test?

Christian

[August Lilleaas]

buster-capture-server provides a test helper module, you should use it to capture slaves.

require("buster-capture-server").testHelper.captureSlave(port, userAgent).then(function (e) {
    e.slave;
    e.slaves;
});

The list of slaves in this promise has the same source as the list of slaves for someServerProcess.slaves(), so it should work.

However, the pubsub client that is created for the non-browser slave is disconnected before the promise for captureSlave() on the test helper is resolved. This will cause the server to consider the slave a "closed browser" in some asynchronous future after the resolve, so you can't reliably do asynchronous stuff and later check .slaves() on the server, it might be gone.

One fix would  be to just use e.slaves, which is a list of the slaves at the time of capture.

Another would be to resolve the captureSlave() promise with a function you can call to manually "close" the mock browser.

[Christian Johansen]

>
> buster-capture-server provides a test helper module, you should use it to capture slaves.

Forgot to mention - this is what I'm using.

>
> The list of slaves in this promise has the same source as the list of slaves for someServerProcess.slaves(), so it should work.
>
> However, the pubsub client that is created for the non-browser slave is disconnected before the promise for captureSlave() on the test helper is resolved. This will cause the server to consider the slave a "closed browser" in some asynchronous future after the resolve, so you can't reliably do asynchronous stuff and later check .slaves() on the server, it might be gone.

That explains it then. My expectation was that captureSlave would capture a persistent slave.

>
> One fix would  be to just use e.slaves, which is a list of the slaves at the time of capture.

I'm integration testing this via the webpage buster-server-cli serves, so that won't work.

>
> Another would be to resolve the captureSlave() promise with a function you can call to manually "close" the mock browser.

That sounds like it would do the trick. Just to be clear - you're thinking of changing the helper now, right?

C

[August Lilleaas]

On Sun, Oct 14, 2012 at 11:17 AM, Christian Johansen <chri...@cjohansen.no> wrote:

>
> buster-capture-server provides a test helper module, you should use it to capture slaves.

Forgot to mention - this is what I'm using.

>
> The list of slaves in this promise has the same source as the list of slaves for someServerProcess.slaves(), so it should work.
>
> However, the pubsub client that is created for the non-browser slave is disconnected before the promise for captureSlave() on the test helper is resolved. This will cause the server to consider the slave a "closed browser" in some asynchronous future after the resolve, so you can't reliably do asynchronous stuff and later check .slaves() on the server, it might be gone.

That explains it then. My expectation was that captureSlave would capture a persistent slave.

>
> One fix would  be to just use e.slaves, which is a list of the slaves at the time of capture.

I'm integration testing this via the webpage buster-server-cli serves, so that won't work.

>
> Another would be to resolve the captureSlave() promise with a function you can call to manually "close" the mock browser.

That sounds like it would do the trick. Just to be clear - you're thinking of changing the helper now, right?

Yeah, change it so the "e" looks like: {teardown: function(){}, e: {slave: {}, slaves: [..]}}.

Just pushed this change :)
 

[Christian Johansen]

Cool, thanks.

Btw, I merged your changes into the 0.7 branch, where (almost) everything is ready for Buster 0.7. Would rock if you based further work off of that branch instead of the error-handling one.

C

[Christian Johansen]

Btw, the helper worked nicely :)

Btw, is this slightly strange?

captureSlave().then(function(e) {
    e.e.slave // ??
})

First: What to call the argument? Second: Is `.e` meaningful? What about:

captureSlave().then(function(capture) {
    capture.slave
    capture.slaves
    capture.kill()
})

I guess calling it "capture" doesn't really cut it as it also has the slaves array... But it's a test helper, I guess convenience trumps "conceptual purity". Anyway, suggestion is to have slave and teardown/kill on the same level. I also suggest renaming "teardown" to "kill", as in "kill the browser - not to be confused with tearing down the test case itself".

Waddaya think?

C

--
MVH
Christian

[August Lilleaas]

I tend to name event objects e, and since it proxies another event object it becomse e.e.... But I might as well put it on the event itself. I'd prefer a more generic name though so users are free to choose another name than "capture" for the argument. teardownSlave() seems like a nice symmetric name to the .captureSlave() call to create it.

Anyone disagree?

[Christian Johansen]

releaseSlave?