Skip to content

Commit

Permalink
Fixes Firefox Nightly and adds improve safari support (#109)
Browse files Browse the repository at this point in the history
* Fixes Firefox Nightly and adds improve safari support

* Adding logging for trqvis debugging

* Force download on travis
  • Loading branch information
Matt Gaunt authored Jan 27, 2018
1 parent 26b480d commit 7794a60
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 28 deletions.
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"main": "src/index.js",
"scripts": {
"lint": "eslint '.'",
"test": "npm run lint && mocha",
"test": "mocha && npm run lint",
"istanbul": "npm run lint && istanbul cover _mocha",
"coveralls": "cat ./coverage/lcov.info | coveralls",
"postpublish-release": "track-pkg-size",
Expand Down Expand Up @@ -47,7 +47,6 @@
},
"dependencies": {
"chalk": "^2.1.0",
"del": "^3.0.0",
"dmg": "^0.1.0",
"fs-extra": "^4.0.2",
"mkdirp": "^0.5.1",
Expand Down
5 changes: 4 additions & 1 deletion src/browser-models/local-browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ class LocalBrowser extends Browser {

const minVersion = this._getMinSupportedVersion();
if (minVersion) {
return this.getVersionNumber() >= minVersion;
const browserVersion = this.getVersionNumber();
if (browserVersion !== -1 && browserVersion < minVersion) {
return false;
}
}

if (this.isBlackListed()) {
Expand Down
12 changes: 6 additions & 6 deletions src/download-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const path = require('path');
const fs = require('fs');
const request = require('request');
const mkdirp = require('mkdirp');
const del = require('del');
const dmg = require('dmg');
const fse = require('fs-extra');
const yauzl = require('yauzl');
Expand Down Expand Up @@ -70,6 +69,7 @@ class DownloadManager {
let localStorage = null;

return new Promise((resolve, reject) => {
// TODO: Switch to fs-extra
mkdirp(localstoragePath, (err) => {
if (err) {
return reject(err);
Expand Down Expand Up @@ -268,7 +268,7 @@ class DownloadManager {
})
.then((filePath) => {
if (filePath) {
return del(filePath, {force: true});
return fse.remove(filePath);
}
});
}
Expand All @@ -295,7 +295,7 @@ class DownloadManager {
ffProduct = 'firefox-beta-latest';
break;
case 'unstable':
firefoxMacApp = 'FirefoxNightly.app';
firefoxMacApp = 'Firefox Nightly.app';
ffProduct = 'firefox-nightly-latest';
break;
default:
Expand Down Expand Up @@ -396,7 +396,7 @@ class DownloadManager {
})
.then((filePath) => {
if (filePath) {
return del(filePath, {force: true});
return fse.remove(filePath);
}
});
}
Expand Down Expand Up @@ -604,7 +604,7 @@ class DownloadManager {
});
})
.then((filePath) => {
return del(currentAppPath, {force: true})
return fse.remove(currentAppPath)
.then(() => filePath);
});
});
Expand All @@ -614,7 +614,7 @@ class DownloadManager {
})
.then((filePath) => {
if (filePath) {
return del(filePath, {force: true});
return fse.remove(filePath);
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ class SeleniumAssistant {
quitTimeout = setTimeout(resolve, 2000);

driver.close()
.then(() => driver.quit())
.then(() => driver.quit(), () => driver.quit())
.then(resolve, resolve);
})
.then(() => {
Expand Down
4 changes: 2 additions & 2 deletions src/local-browsers/firefox.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class LocalChromeBrowser extends LocalBrowser {
// Find OS X expected path
let firefoxAppName;
if (this._release === 'unstable') {
firefoxAppName = 'FirefoxNightly.app';
firefoxAppName = 'Firefox Nightly.app';

This comment has been minimized.

Copy link
@sylvestre

sylvestre Jan 29, 2018

I don't think this is valid. We reverted this renaming a few weeks ago.
This will probably fail.

This comment has been minimized.

Copy link
@gauntface

gauntface Jan 29, 2018

It's working at the moment (i.e. the current downloading of Firefox Nightly).

This comment has been minimized.

Copy link
@gauntface

gauntface Jan 29, 2018

@sylvestre do you know what this will change will start landing in downloads?

} else {
firefoxAppName = 'Firefox.app';
}
Expand Down Expand Up @@ -129,7 +129,7 @@ class LocalChromeBrowser extends LocalBrowser {
if (this._release === 'stable') {
return '/Applications/Firefox.app/Contents/MacOS/firefox';
} else if (this._release === 'unstable') {
return '/Applications/FirefoxNightly.app/Contents/MacOS/firefox';
return '/Applications/Firefox Nightly.app/Contents/MacOS/firefox';

This comment has been minimized.

Copy link
@sylvestre

sylvestre Jan 29, 2018

Ditto

}
break;
case 'linux':
Expand Down
1 change: 1 addition & 0 deletions src/local-browsers/safari.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class LocalSafariBrowser extends LocalBrowser {
*/
getSeleniumDriverBuilder() {
const seleniumOptions = this.getSeleniumOptions();
seleniumOptions.setTechnologyPreview((this._release === 'beta'));

const builder = new webdriver
.Builder()
Expand Down
27 changes: 16 additions & 11 deletions test/browser-usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,14 @@ describe('Test Usage of Browsers', function() {
const versionString = specificBrowser.getRawVersionString();
(typeof versionString).should.equal('string');
(versionString === null).should.equal(false);
versionString.length.should.gt(0);
// Chrome unstable has made it possible that this could happen
// versionString.length.should.gt(0);

const versionNumber = specificBrowser.getVersionNumber();
(typeof versionNumber).should.equal('number');
versionNumber.should.not.equal(-1);
if (versionString) {
versionNumber.should.not.equal(-1);
}

const prettyName = specificBrowser.getPrettyName();
prettyName.length.should.gt(1);
Expand All @@ -109,9 +112,9 @@ describe('Test Usage of Browsers', function() {
return;
}

testBrowserInfo(localBrowser);
return testNormalSeleniumUsage(localBrowser)
.then(() => testBuilderSeleniumUsage(localBrowser))
.then(() => testBrowserInfo(localBrowser));
.then(() => testBuilderSeleniumUsage(localBrowser));
});

it('should get null for raw version output if no executable found', function() {
Expand Down Expand Up @@ -161,14 +164,15 @@ describe('Test Usage of Browsers', function() {
before(function() {
seleniumAssistant.setBrowserInstallDir(null);

const expiration = process.env.TRAVIS ? 0 : 24;
console.log('Downloading browsers....');
return Promise.all([
seleniumAssistant.downloadLocalBrowser('chrome', 'stable'),
seleniumAssistant.downloadLocalBrowser('chrome', 'beta'),
seleniumAssistant.downloadLocalBrowser('chrome', 'unstable'),
seleniumAssistant.downloadLocalBrowser('firefox', 'stable'),
seleniumAssistant.downloadLocalBrowser('firefox', 'beta'),
seleniumAssistant.downloadLocalBrowser('firefox', 'unstable'),
seleniumAssistant.downloadLocalBrowser('chrome', 'stable', expiration),
seleniumAssistant.downloadLocalBrowser('chrome', 'beta', expiration),
seleniumAssistant.downloadLocalBrowser('chrome', 'unstable', expiration),
seleniumAssistant.downloadLocalBrowser('firefox', 'stable', expiration),
seleniumAssistant.downloadLocalBrowser('firefox', 'beta', expiration),
seleniumAssistant.downloadLocalBrowser('firefox', 'unstable', expiration),
])
.catch((err) => {
console.warn('There was an issue downloading the browsers: ', err);
Expand Down Expand Up @@ -200,7 +204,8 @@ describe('Test Usage of Browsers', function() {
return globalServer.killServer();
});

const localBrowserFiles = fs.readdirSync('./src/local-browsers');
const localBrowserFiles = fs.readdirSync(
path.join(__dirname, '..', 'src', 'local-browsers'));
localBrowserFiles.forEach((localBrowserFile) => {
const LocalBrowserClass = require(`./../src/local-browsers/${localBrowserFile}`);
const browserReleases = LocalBrowserClass.getPrettyReleaseNames();
Expand Down
4 changes: 2 additions & 2 deletions test/downloading-browsers.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const del = require('del');
const fs = require('fs-extra');
const seleniumAssistant = require('../src/index.js');

const TIMEOUT = 5 * 60 * 1000;
Expand All @@ -14,7 +14,7 @@ describe('Test Download Manager - Browser Download', function() {
// Reset Install Directory
seleniumAssistant.setBrowserInstallDir(null);

return del(seleniumAssistant.getBrowserInstallDir(), {force: true});
return fs.remove(seleniumAssistant.getBrowserInstallDir());
});

const setupDownloadTest = (browserId, release) => {
Expand Down
6 changes: 3 additions & 3 deletions test/expiration-behavior.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const del = require('del');
const fse = require('fs-extra');
const sinon = require('sinon');
const LocalStorage = require('node-localstorage').LocalStorage;
const path = require('path');
Expand Down Expand Up @@ -149,14 +149,14 @@ describe('Test Download Manager - Browser Expiration', function() {
stub.restore();
});

return del(seleniumAssistant.getBrowserInstallDir(), {force: true});
return fse.remove(seleniumAssistant.getBrowserInstallDir());
});

beforeEach(function() {
this.timeout(6000);

// Ensure the test output is clear at the start
return del(seleniumAssistant.getBrowserInstallDir(), {force: true});
return fse.remove(seleniumAssistant.getBrowserInstallDir());
});

const browsers = [
Expand Down

0 comments on commit 7794a60

Please sign in to comment.