Bug with webdriver's inheritance about wd HOT 5 CLOSED

This looks totally right to me -- if you were up for making the changes and submitting a pull request, I would be happy merge it in! Thanks.

from wd.

No, it is wrong. It would be right if you were to write

webdriver.prototype = EventEmitter.prototype;

Then you would still have to call the EventEmitter constructor in the webdriver constructor.
new EventEmitter(); creates an instances, you can't inherit instance of object. Well you can obviously, but is wrong and doesn't work as it should except in the case where you use one webdriver instance in the lifetime of the app.

I fixed it myself and as you can see above the fix is three lines long and I tested it. It would take you less time to copy paste it than for me to fork your repository, fix, push, submit pull request, then you need to merge the changes, etc. Just copy the lines above. Or here is the whole begining of the file webdriver.js

var EventEmitter = require('events').EventEmitter;
var async = require("async");
var _ = require("underscore");
var fs = require("fs");
var element = require('./element').element;
var request = require('request');

var __slice = Array.prototype.slice;
var utils = require("./utils");
var JSONWIRE_ERRORS = require('./jsonwire-errors.js');
var MAX_ERROR_LENGTH = 500;
var util = require( 'util' );

// webdriver client main class
// WdConfig is an option object containing the following fields:
// host,port, username, accessKey
var webdriver = module.exports = function(remoteWdConfig) {
  EventEmitter.call( this );
  this.sessionID = null;
  this.username = remoteWdConfig.username || process.env.SAUCE_USERNAME;
  this.accessKey = remoteWdConfig.accessKey  || process.env.SAUCE_ACCESS_KEY;
  this.basePath = (remoteWdConfig.path || '/wd/hub');
  this.https = (remoteWdConfig.https || false);
  // default
  this.options = {
    host: remoteWdConfig.host || '127.0.0.1'
    , port: remoteWdConfig.port || 4444
    , path: (this.basePath + '/session').replace('//', '/')
  };
  this.defaultCapabilities = {
    browserName: 'firefox'
     , version: ''
    , javascriptEnabled: true
    , platform: 'ANY'
  };
  // saucelabs default
  if ((this.username) && (this.accessKey)) {
    this.defaultCapabilities.platform = 'VISTA';
  }
};

//inherit from EventEmitter
util.inherits( webdriver, EventEmitter );

from wd.

I agree that this a better way to do this, however having applied your changes the wd tests are now breaking:

https://travis-ci.org/admc/wd/builds/5824508

I don't have time at the moment to dive in and debug this, any thoughts would be appreciated.

I'm running 'make test' locally with selenium 2.31: http://docs.seleniumhq.org/download/
and 'make test_saucelabs' the way travis is and seeing the same error.

Adam

from wd.

I tried to spot something but couldn't find anything that doesn't look right, sorry. I'm still new to WebDriver. Maybe something will popup when I start using it soon.

from wd.

Made the change, @admc, it looks like you had messed up the wd remote configuration (plus there was a missing callback in one of the new test).

from wd.