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

as a developer i want to run my JS tests using phantomJS (or similar headless browser) so they run faster #105

Open
andy-berry-dev opened this issue Oct 8, 2013 · 26 comments
Assignees

Comments

@andy-berry-dev
Copy link
Member

alot of testing libraries are using things like phantomJS to run JS tests. it would be nice if we can support headless browsers and use these by default in the BRJS build - we can still use 'real' browsers in CI.

(this may already be possible, noting it down so we can keep track of it)

@andy-berry-dev
Copy link
Member Author

Jan has done a spike with this and seems to have had good results. Key files below, original src at https://stash.caplin.com/projects/TP/repos/cutlass_phantomjs/browse

phantom_browser.js

var page = require('webpage').create();

page.settings.userAgent = 'PhantomJS test slave';

page.open('http://localhost:4224/capture', function(status) {
    if (status !== 'success') {
        console.log('Could not connect to JSTD');
        phantom.exit();
    }

    console.log('Loaded JSTD page');
});

README.md

# Headless testing of cutlass with PhantomJS

## Setup

1. Install [PhantomJS](http://phantomjs.org/download.html) (or use the binaries provided in this repo*, win binary hasn't been tested)
2. Edit your `test-runner.conf` and add phantomjs to the list of available browsers:

` ``
browserPaths:
  windows:
    ...
  mac:
    ...
    phantomjs: /path/to/phantomjs$$/path/to/phantom_browser.js
    ...
` ``

Change the paths off course.

3. Change the `defaultBrowser` to `phantomjs`
4. Start the test server: `./bladerunner test-server`
5. Run the tests: `./bladerunner test /path/to/tests`
6. Rejoice!

_* I know you shouldn't do that, this is for your convenience :)_

@andy-berry-dev
Copy link
Member Author

AndyL has also used PhantomJS with BRJS:

PhantomJS is a headless web-kit which is useful for running things like unit tests without starting up a whole browser. We have at least one client who's using it & it's compatible with CT3/bladrunner so here's some steps to get it working:

  • Install PhantomJS on your machine
  • Add a config for PhantomJS to the test-runner.conf file. Replace phantomjs_path with the path to the phantomjs executable (or if it's on your path, just phantomjs will do)
phantomjs: phantomjs_path$$..\conf\phantomjs-runner.js
  • Download the phantomjs-runner.js file attached to this page and place it in the /conf folder, next to the test-runner.conf file
  • You should be able to run unit tests in PhantomJS by passing the -b phantomjs flag into the bladerunner test command

Credit:
PhantomJS runner file a modified version of the one from here:
https://github.com/larrymyers/js-test-driver-phantomjs/blob/master/phantomjs-jstd.js

PhantomJS runner:

var page = require('webpage').create();
var args =  require('system').args;
var url = args[1];
var captureAttempts = 0;
var captured = false;
var locked = false;

var log = function(str) {
    var dt = new Date();
    console.log(dt.toString() + ': ' + str);
};

var pageLoaded = function(status) {
    log('Finished loading ' + url + ' with status: ' + status);

    var runnerFrame = page.evaluate(function() {
        return document.getElementById('runner');
    });

    if (!runnerFrame) {
        locked = false;
        setTimeout(capture, 1000);
    } else {
        captured = true;
    }
};

var capture = function() {
    if (captureAttempts === 5) {
        log('Failed to capture JSTD after ' + captureAttempts + ' attempts.');
        phantom.exit();
    }

    if (captured || locked) {
        return;
    }

    captureAttempts += 1;
    locked = true;

    log('Attempting (' + captureAttempts + ') to load: ' + url);
    page.open(url, pageLoaded);
};

capture();

@james-shaw-turner
Copy link
Contributor

Nice work everyone :-) we can put this in once we get CT integration out the door.

@leggetter
Copy link
Contributor

I'd like to see us addressing this with a Java-focus. We know there are tools available in the node.js world and integration should be relatively easy. But what about those that don't and can't use node.js?

@andy-berry-dev
Copy link
Member Author

I dont think this is Node based, it looks likes its a native binary. From http://phantomjs.org/download.html

Download phantomjs-1.9.7-windows.zip (6.7 MB) and extract (unzip) the content.

The executable phantomjs.exe is ready to use.

@janhancic
Copy link
Contributor

It's not node.

@leggetter
Copy link
Contributor

Ok, my bad.

I'd like to see us addressing this with a Java-focus

I stand by this.

@andy-berry-dev andy-berry-dev changed the title as a developer i want to run my JS tests using phantomJS (or similar headless browser) so they run quicker as a developer i want to run my JS tests using phantomJS (or similar headless browser) so they run faster Jun 10, 2014
@lukeapage
Copy link
Contributor

Hi,

We already use this method with the non-open-source version of bladerunner. We find that depending on the amount of tests and the computer it is run on, bladerunner (or js test driver) hangs very often. I suspect it is js test driver and there is nothing you can do about it, but something to think about.

Once CT is using this project I can report back on whether it is still hanging (on our bamboo CI server it hangs 1 in 3-5 builds). I uploaded logs to SDANSDPL-158 if they bear any relation.

I'm also hoping that its js-test-driver and that switching to karma may allow us to do headless testing without hangs.

@dchambers
Copy link
Contributor

Hi @lukeapage, yes the hanging is definitely js-test-driver related, as we've also seen the same thing when running the tests directly with js-test-driver (no BladeRunner involved). I've met a couple of people in the past who abandoned js-test-driver in favor of Karma for exactly this reason, and we need to do the same.

@lukeapage
Copy link
Contributor

Great, sounds like a plan.

@andy-berry-dev
Copy link
Member Author

Once #857 is merged in (probably ready for a v0.12 release) PhantomJS will be part of the standard BRJS release and will be configured to be the default browser.

@andy-berry-dev andy-berry-dev added this to the v0.12 milestone Jul 29, 2014
@thecapdan
Copy link
Contributor

Just gave this a go. Works nicely. But...

one issue: when running without a browser the stack trace for failing tests is not as useful. e.g.

phantom mode:

PhantomJS  : Run 124 tests (Passed: 123; Fails: 1; Errors 0) (6107.00 ms)
    behaviour of EditableProperty objects in a Presentation Model. parses and formats value entered via input box
         failed (63.00 ms): AssertError: AssertError: "value" should be d expected "d" but was "C" in http://localhost:4224/static/runner.js (line 356)
            AssertError: "value" should be d expected "d" but was "C"
            at bundles/js/js.bundle:86238
            at bundles/js/js.bundle:84990
            at bundles/js/js.bundle:84771
            at tests/property/EditablePropertyTest.js:62
            at bundles/js/js.bundle:3607
            at bundles/js/js.bundle:4396
            at bundles/js/js.bundle:4349
            at bundles/js/js.bundle:4669
            at bundles/js/js.bundle:4396
            at bundles/js/js.bundle:4392
            at bundles/js/js.bundle:4641
            at bundles/js/js.bundle:4668
            at bundles/js/js.bundle:4406
            at bundles/js/js.bundle:4386

with chrome:

Chrome 23.0.1235.0 Windows: Run 124 tests (Passed: 123; Fails: 1; Errors 0) (4211.00 ms)
    behaviour of EditableProperty objects in a Presentation Model. parses and formats value entered via input box
         failed (39.00 ms): AssertError: AssertError: "value" should be d expected "d" but was "C"
            AssertError: "value" should be d expected "d" but was "C"
            at ViewFixture.doThen (bundles/js/js.bundle:86235:3)
            at GwtTestRunner.doThen (bundles/js/js.bundle:84990:22)
            at bundles/js/js.bundle:84771:23
            at null.<anonymous> (tests/property/EditablePropertyTest.js:62:3)
            at jasmine.Block.execute (bundles/js/js.bundle:3607:15)
            at jasmine.Queue.next_ (bundles/js/js.bundle:4396:31)
            at jasmine.Queue.start (bundles/js/js.bundle:4349:8)
            at jasmine.Spec.execute (bundles/js/js.bundle:4667:14)

One other thing, phantomJS adds 50 meg to our distribution zip:( discuss

@andy-berry-dev
Copy link
Member Author

one issue: when running without a browser the stack trace for failing tests is not as useful. e.g.

I don't think theres much we can do here given it's the browser generating the call stack. IMO its a reasonably minor thing since its easy to run the tests in another browser to debug.

phantomJS adds 50 meg to our distribution zip:( discuss

Pros: we have a default browser which massively simplifies the onboarding process (keeping @leggetter happy 😄)
Cons: we're now back up around 50meg for the download

IMO this isn't a huge deal since BRJS only needs to be downloaded once (for a given version) and nowadays people have a good enough connection to download 100meg.

@leggetter
Copy link
Contributor

We should probably have something somewhere which states what's in the zip. On the FAQ page? "Why is BRJS ~100MB?".

If we get complaints about the size we could look at alternatives e.g.:

  1. Have two zips - one with and one without PhantomJS
  2. Have BRJS prompt to see if the user wishes to download and use PhantomJS

@thecapdan
Copy link
Contributor

@andyberry88 some banks have 100meg single download restrictions, so to get this zip they'll need to go through red tape or resort to sneaky means to download it in chunks. So pushing our dist zip back over 100meg may reduce our 'blue chip' trialists

@dchambers
Copy link
Contributor

So just to be clear, it's slower, has less readable call stacks, makes BRJS less portable (won't work on Solaris or BSD for example, I'm just saying), doesn't have any debugging tools, and increases our download size, for the single benefit that it doesn't open up a window while test are running?

@andyberry88 is there a reason why in the past we ddn't distribute Chromium, since that would also have solved the usability problem of having to configure a browser before running tests?

@dchambers
Copy link
Contributor

@thecapdan oh yeah, I'd forgetten about that. That used to happen at a bank where I was working too, so that installing Eclipse JEE was a major faff.

@andy-berry-dev
Copy link
Member Author

it's slower

I don't agree with this, I've seen tests execute ~ 15% faster.

has less readable call stacks

Yep, I'll accept that one.

won't work on Solaris or BSD for example

I doubt this is something we're ever going to have issues with.....

The fact that it doesn't open a separate window is a massive win. The reason we haven't shipped with other browsers (and that most can be unreliable) is they all have local profiles that are used. This means any crashes that happen in one session can ofter spill over (warnings etc) in to the next session. We don't see this issue with our build since it forces the browsers to use fresh profiles which the build recreates each time - this isn't something a general user of BRJS will have.

some banks have 100meg single download restrictions

I can see this as an issue. Perhaps we need 2 distributions as @leggetter suggested. I don't think the prompt to download would work since that would presumably be an issue for banks too.
The other option would be to print a warning that gives the URL where a version of PhantomJS can be downloaded.

@lukeapage
Copy link
Contributor

IMO you don't want to include phantomjs. What happens when a new version of phantomjs comes out? If i update BRJS but phantomjs hasn't updated do I want to have to re-download it? For people who would check-in BRJS as part of the development environment, should they have to check in phantomjs too?

If BRJS was a node package you would have dependencies that would just install (there is a node phantom package which includes the latest phantom) - have you thought about using some other package manager to enable users to configure and download multiple dependencies without including it all in one zip?

w.r.t.stack traces - we don't find it a problem, we use phantom for our CI server and locally to avoid having to give chrome focus (because we have a few tests that rely on focus) but if we get a problem that means we need to debug we switch to chrome. Very rarely will a problem in tests be made more obvious by the stack and not involve debugging.

For our CI server - we wish we didn't use phantom, because we now have over 3000 uts and ats and it means phantom fails over half the time. We're looking forward to getting support for karma.

Have you had any luck running chrome as a service with bladerunner as an alternative to phantom in CI builds?

@dchambers
Copy link
Contributor

Hi @lukeapage, lot's of good points here.

One point of confusion though; can you please provide more background detail as to why we might want to run Chrome as a service to be able to use Chrome on CI? We've been running Chromium, Firefox & IE on our CI boxes since the beginning without incident. We originally tried Chrome instead of Chromium but it caused issues on developer boxes where Chrome might already have been running, but I can't remember there being any other issues with it.

@andy-berry-dev
Copy link
Member Author

(Damn, @dchambers beat me to it 😄)

Thanks for the input @lukeapage, I'm not sure what the answer is here since IMO there are huge pros and cons either way. One of the problems we've had up until now is we've found it difficult to provide a template test-runner.conf with accurate browser paths since they could be wildly different on each developer machine. This isn't a problem for developers comfortable with using BRJS but it's another step in the on-boarding process and our getting started guide then has another concept to introduce. If we shipped with a browser where we knew the path running tests would work out of the box.

What happens when a new version of phantomjs comes out?

Since it's just a test runner configuration the path to the PhantomJS could be updated to use a locally installed version. This would be the same if we updated PhantomJS and a developer prefered to stay on the version they were using previously.

For people who would check-in BRJS as part of the development environment, should they have to check in phantomjs too?

If BRJS is checked in I'd imagine they would also include PhantomJS. This isn't ideal and IMO we should remove the need for BRJS to be checked in at all. One of my pet hates is binaries checked in to source control but unfortunately that's the easiest way to make it work at the moment. One way to go with this would be to do something similar to Gradle where they have a Gradle wrapper (http://www.gradle.org/docs/current/userguide/gradle_wrapper.html). The wrapper itself and a small amount of config is checked in and it deals with downloading the required version of Gradle. This makes it very easy to switch between Gradle versions and test updates.

have you thought about using some other package manager to enable users to configure and download multiple dependencies without including it all in one zip?

Integrating package managers in to BRJS is one thing we hope to look at in the coming months although we'd thought about making use of it for library dependencies (jQuery, Knockout etc.). Using it as part of core BRJS to allow commands and plugins to download dependencies is an interesting idea.


For our CI server - we wish we didn't use phantom, because we now have over 3000 uts and ats and it means phantom fails over half the time.
Have you had any luck running chrome as a service with bladerunner as an alternative to phantom in CI builds?

One of the aims for the build is to keep the dev build as close as possible to the CI build. We've had a huge amount of pain in the past where the only way to debug something was to go on to the CI box which in turn caused more problems for builds running after.
Our build, and most of our JS builds internally, download the browsers from a Maven repo and put them in the right place on disk. The task that copies the browsers also creates a fresh profile for each browser (copyBrowsers task in https://github.com/BladeRunnerJS/brjs/blob/master/brjs-sdk/build.gradle) in order to prevent all sorts of popups appearing when the browser first starts. BRJS then deals with starting and stopping the browser and the config in test-runner.conf specifies the correct flags so the browsers use the given profiles and run in incognito mode (https://github.com/BladeRunnerJS/brjs/blob/master/brjs-sdk/build-resources/test-runner.conf).
The only browser we've had problems with using this approach is IE, particularly IE8. All CI machines have various registry hacks to suppress a lot of the warnings that appear (IE is the only browser install on a build machine) which helps but IE8 occasionally falls over with a large test suite.


General brain dump of where we are so far with the 'is PhantomJS part of the BRJS zip' question:

  • Including PhantomJS makes the download huge, which isn't cool. So whatever we do it looks like we can't include PhantomJS in the core download.
  • If we print a warning with a path to download PhantomJS and a dev doesn't download it we're still left with a broken config and are back to square 1
  • If we try and auto download PhantomJS this isn't something that people would necessarily expect and I imagine would cause problems in banks etc.
  • We don't include PhantomJS at all but include the runner script incase people want to use it.
  • We could produce 2 zips where one doesnt have PhantomJS.

@dchambers
Copy link
Contributor

@andyberry88 so is the reason @lukeapage and other external developers will see greater value to using PhantomJS than we do is because the copyBrowsers task is a gradle task, and not a BRJS command that can be shared with others?

If this was converted into a BRJS command, would not that not also solve the initial download size problem, the browser paths problem and the fact that others are struggling to get browsers reliably working in their CI environments?

For example, to perform a full download the user could type:

brjs test-browsers

which would download the relevant PhantomJs, Chromium and Firefox for their platform, and configure their 'test-runner.conf' to make use of these browsers.

Whereas to refresh the profiles of already downloaded browsers the user could type:

brjs test-browsers --refresh

Users following the tutorials could then be instructed to run this command before attempting to run any tests, and the build scripts people create for their CI could use these scripts to ensure they have correctly configured browsers available to test with.

Finally, developers at banks that may have to connect via a proxy, would be aware that the brjs test-browsers failed when they explicitly ran it, and could make alternative arrangements by configuring the Java control panel to use a proxy, or by downloading these browsers manually.

@lukeapage
Copy link
Contributor

Hrmm, Well I tried it with chrome and it does work (though with the following errors)

browser #1 <stderr>: [11360:12708:0731/113822:ERROR:gpu_info_collector_win.cc(102)] Can't retrieve a valid WinSAT assessment.
browser #1 <stderr>: [11360:12708:0731/113822:ERROR:chrome_views_delegate.cc(176)] NOT IMPLEMENTED
browser #1 <stderr>: [11360:12708:0731/113822:ERROR:desktop_root_window_host_win.cc(746)] NOT IMPLEMENTED

but speaking to the @chrisprice he thought that it didn't work if the account was a proper service account (we are temporarily running our CI server under a normal user account) - it would be good to know if you did or didn't have any problems with this.

@dchambers
Copy link
Contributor

Hi @lukeapage,

Those Chrome errors can be ignored, and I think in the current BRJS code you won't even see those kinds of errors anyway. We use a browser path like this, which uses the --incognito flag:

chrome: ../../build/browsers/chrome-win32/chrome.exe$$--incognito$$--user-data-dir=../../build/browsers/profile/chromium

and perhaps this makes it work regardless of what user is logged in? @andyberry88 might that be true?

@andy-berry-dev andy-berry-dev removed this from the 0.12 milestone Aug 1, 2014
@andy-berry-dev
Copy link
Member Author

and perhaps this makes it work regardless of what user is logged in

It's the --user-data-dir that helps here. This specifies a different directory to use for the browser profile (rather than the default for the current user) which ensures browser settings for an installed Chrome don't clash with Chrome thats run via the build. We previously ran the CI client as a service (It may have changed since I was on the Release Eng. team) and I don't remember seeing any issues running browsers. It could be I'd changed the build to use --user-data-dirto fix unreliability issues on dev machines and thats why it worked when we started running CI as a service.


For the time being we'll remove PhantomJS as a browser shipped with BRJS and just use it within our build. I like the commands suggested by @dchambers but these probably need some thought before we look at tackling them.

Moving back into the backlog, update to follow...

@andy-berry-dev
Copy link
Member Author

Closed in error after merging #939

@andy-berry-dev andy-berry-dev reopened this Sep 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants