search

brzpegasus / ember-cli-nwjs

An addon for building desktop apps with Ember and NW.js
MIT License
125 stars 17 forks source link

feat(tests/infra) - interactive mode implementation #25

Closed sedouard closed 9 years ago

sedouard commented 9 years ago

Creates a 'nw:test --server' switch which starts tests in interactive mode using testem's server mode. Qunit-logger connects to the server via web socket and re-runs tests without restarting the process. Interactive mode shows nwjs where ci mode hides.

sedouard commented 9 years ago

I went with the approach of using the testem server. Basically the idea is that we don't exit the test run from nwjs and instead hook into the server's socket.io api that it exposes for browsers in quint-logger. When the user changes something the watcher will tell testem to instruct the browser to restart the tests which just causes the browser to reload.

Thoughts?

brzpegasus commented 9 years ago

Thanks! I will take a look later this morning.

brzpegasus commented 9 years ago

I've been playing with this locally (still am), but here are some initial thoughts.

First, I appreciate the attempt to have this interact with the testem server. I was originally thinking of just reusing the stuff you had already implemented for ember nw:test and just not exit the child process (in the runner) at the end, if we're running in interactive mode. However, making this work with Testem's GUI is even nicer.

There are a few issues to address before this can be merged, and I'm still trying to figure some of those items out myself:

I need to spend more time with this, but overall, I dig it! We just need to make it work =).

sedouard commented 9 years ago

Sorry for the delay here by the way - I'm traveling about but I'm going to put a couple fixes in the next couple days.

brzpegasus commented 9 years ago

@sedouard No problem! I've been busy as well, but I'm hoping to get back to this and help figure out the remaining items before too long.

sedouard commented 9 years ago 
The tests are running too soon.

Yup - you're right. I took your idea but I added an environment variable to indicate we're in server mode so that we only start the tests on socket connection for server mode tests. Normally I would think that calling the build command with the 'development' environment mode would pass this in to the meta environment data on the built index.html. However it remains test as environment. So I did this as a workaround.

I always get a Failed to load resource: net::ERR_ADDRESS_UNREACHABLE error for http://localhost:7357/socket.io/?EIO=3&transport=polling&t=<some-token> when connecting to the testem server, but the connection does succeed. This might again be related to the server not being ready.

Yeah - but is this that big of a deal? Socket.io's behavior is to retry to connect and it'll try to connect again.

Rather than introducing a new dependency on socket.io-client, couldn't we fetch it from Testem? http://localhost:7357/socket.io/socket.io.js should give us what we need, no?

Agreed - should we get it from bower_components or pull it directly from localhost:7357 at run-time? (Currently this commit I just added does the former)

When the tests are done running, the Testem GUI shows "Waiting for runners..." instead of the results of the tests. But things do show up while the test suite is in progress. Gotta figure out how to make those results stay.

Here's what I see after my tests run: asciicast

The stuff that the QUnit logger is console.log'ging during the tests is showing up in the testem GUI. We'll probably want to turn that off in this mode. It's only needed for CI.

True - this was intentional I didn't think there was any harm in just outputting the qunit-logger's output to the socket-io connection. I can take this out though.

I plugged this into my demo app as I already had a set of NW.js-dependent tests written for. I find that if I enable the "Preserve log" option in the console, and then manually refresh the page to have the tests re-run, the log shows that the entire suite actually executes multiple times. The first time everything does pass (woo!), but then something causes the page to reload and eventually the tests fail with some weird error (some node error about not being able to find a file named 'uv_cwd').

Can you try it with the new changes?

brzpegasus commented 9 years ago

@sedouard Sorry, now I'm delaying! I will check this out soon. The plan is to get this all figured out, merged in, and released by June 16th, because guess who's giving a talk at this year's Wicked Good Ember conference? =) It would be great to share some of this work.

sedouard commented 9 years ago

Excellent and congrats! Looks like we have a deadline :-).

I haven't ran the PR o your ember project - hoping it solves the test timing issue. I'm hitting a small issue with number of tests reported by qunit maybe or maybe that doesn't reproduce with you.

brzpegasus commented 9 years ago

When the tests are done running, the Testem GUI shows "Waiting for runners..." instead of the results of the tests. But things do show up while the test suite is in progress.

I checked out the video you linked, and I do get the same behavior, but only when tests pass. Have you been able to check what happens on your end when you have a failing test? That's when the testem GUI reloads to say "Waiting for runners..."

sedouard commented 9 years ago

so testem has exactly what we want which does the interaction with the socket.io api correctly. So I based qunit-logger off of it with the current tap output today for non-interactive mode runs

Can you try this one out?

brzpegasus commented 9 years ago

It is better in that when tests fail, Testem's console does reflect that, instead of showing "Waiting for runners...".

There's still an issue with the tests running twice. You can see that the page reloads in the NW.js window during the test suite execution, after it finishes (and passes) the first time. The second time that the tests re-run, they fail with some Node error about not being able to find uv_cwd. I can look more into it.

In the meantime, there is some refactoring I'd like to see in this PR to clean things up a bit. I'll start adding some inline comments as I go through the code.

brzpegasus commented 9 years ago

There's still an issue with the tests running twice. You can see that the page reloads in the NW.js window during the test suite execution, after it finishes (and passes) the first time. The second time that the tests re-run, they fail with some Node error about not being able to find uv_cwd. I can look more into it.

Ok, so I figured out what was causing the reload. Some of my acceptance tests were creating files into directories that were being watched, so of course, they re-triggered the test suite. /facepalm

brzpegasus commented 9 years ago

@sedouard I played with this a bunch, and I find that after making the adjustments that I suggested, the QUnit.config.autostart = false and QUnit.start() are no longer necessary. In fact, leaving them is causing the tests to no longer run atomically for reasons I'm a bit unclear on (i.e. the testStart hook for test 2 would execute before the testDone hook for test 1).

There are some tweaks to make to the QUnit logger to make it work with the non-interactive test mode and custom TAP reporter again (this PR breaks the current behavior as details of failed tests are no longer displayed in CI mode). However, I'll be happy to make those tweaks after I merge this in.

So, I would say the only things holding back this PR are the comments I made. Once those are addressed, we should be good to go (and I did verify locally that with the changes I suggested, everything does work perfectly). Getting close!

rwjblue commented 9 years ago

@brzpegasus - I am not 100% sure if it is related, but ember-cli-qunit does "things" with autostart (to help mitigate the sourcemap issue). See here.

brzpegasus commented 9 years ago

@rwjblue Interesting! Looks like that is related then, because if I re-enable QUnit.config.autostart = false and QUnit.start() from this addon, and comment them out in ember-cli-qunit, then tests run fine again. Having them in both places is problematic.

brzpegasus commented 9 years ago

@sedouard Thank you very much!

sedouard commented 9 years ago

:+1: :-)