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.

Sign up for GitHub

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

Jump to bottom

Removed the BrowserStack integration from ckeditor5-dev repository #524

Merged
merged 1 commit into from
Jun 6, 2019
Merged

Removed the BrowserStack integration from ckeditor5-dev repository #524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 0 additions & 22 deletions packages/ckeditor5-dev-tests/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,6 @@ You can also use the bin script for testing a package:
* `browsers` - Browsers which will be used to run the tests. Also available as an alias: `-b`.
* `reporter` - Mocha reporter – either `mocha` (default) or `dots` (less verbose one).

#### BrowserStack

You can use BrowserStack to execute the tests on different browsers. At this moment we have defined 2 browsers:

1. Edge 16 - Windows 10 (`BrowserStack_Edge`)
1. Safari 11 - OS X High Sierra (`BrowserStack_Safari`)

In order to run BrowserStack, you need to specify the environment variables: `BROWSER_STACK_USERNAME` and `BROWSER_STACK_ACCESS_KEY`.
By default, the tests will be executed on Chrome that is installed on your machine. You can use an option `--browsers` to specify the browsers.

#### Examples

Test the `ckeditor5-enter` and `ckeditor5-paragraph` packages and generate code coverage report:
Expand Down Expand Up @@ -97,18 +87,6 @@ $ npm t -- --files='!(engine)'
$ npm t -- --files='!(engine|ui)'
```

Test the `ckeditor5-engine` using BrowserStack on Edge:

```bash
$ BROWSER_STACK_USERNAME=[...] BROWSER_STACK_ACCESS_KEY=[...] npm test -- --files engine --browsers=BrowserStack_Edge
```

Test the `ckeditor5-image` and `ckeditor5-ui` using BrowserStack on Safari and Edge:

```bash
$ BROWSER_STACK_USERNAME=[...] BROWSER_STACK_ACCESS_KEY=[...] npm test -- --files image,ui --browsers=BrowserStack_Safari,BrowserStack_Edge
```

### Rules for converting `--files` option to glob pattern:

| `--file` | Glob | Description |
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-dev-tests/bin/test-travis.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ yarn run lint && \
${ROOT_BIN}/ckeditor5-dev-tests-check-dependencies && \
cd ${CKEDITOR5_TEST_ENVIRONMENT} && \
node --max_old_space_size=4096 $ROOT_BIN/ckeditor5-dev-tests --files=$PACKAGE_NAME --coverage --reporter=dots --browsers=Chrome && \
node --max_old_space_size=4096 $ROOT_BIN/ckeditor5-dev-tests --files=$PACKAGE_NAME --reporter=dots --browsers=Firefox,BrowserStack_Edge,BrowserStack_Safari
node --max_old_space_size=4096 $ROOT_BIN/ckeditor5-dev-tests --files=$PACKAGE_NAME --reporter=dots --browsers=Firefox
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or Firefox,Chrome?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And in the https://github.com/ckeditor/ckeditor5-react/pull/95/files we can have Chrome too I guess.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we shouldn't. We want to generate CC report only once. If we merge those lines, we will generate two reports.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so that's fine for me.

76 changes: 6 additions & 70 deletions packages/ckeditor5-dev-tests/lib/utils/automated-tests/getkarmaconfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,19 +86,6 @@ module.exports = function getKarmaConfig( options ) {
CHROME_LOCAL: {
base: 'Chrome',
flags: [ '--disable-background-timer-throttling', '--js-flags="--expose-gc"' ]
},
BrowserStack_Edge: {
base: 'BrowserStack',
os: 'Windows',
os_version: '10',
browser: 'edge',
browser_version: '17.0'
},
BrowserStack_Safari: {
base: 'BrowserStack',
os: 'OS X',
os_version: 'Mojave',
browser: 'safari'
}
},

Expand Down Expand Up @@ -128,17 +115,6 @@ module.exports = function getKarmaConfig( options ) {
delete karmaConfig.webpackMiddleware.stats;
}

if ( shouldEnableBrowserStack() ) {
karmaConfig.browserStack = {
username: process.env.BROWSER_STACK_USERNAME,
accessKey: process.env.BROWSER_STACK_ACCESS_KEY,
build: getBuildName(),
project: 'ckeditor5'
};

karmaConfig.reporters = [ 'dots', 'BrowserStack' ];
}

if ( options.coverage ) {
karmaConfig.reporters.push( 'coverage' );

Expand Down Expand Up @@ -176,51 +152,11 @@ function getBrowsers( options ) {
return null;
}

const browsers = options.browsers
.map( browser => {
if ( browser !== 'Chrome' ) {
return browser;
}

return process.env.TRAVIS ? 'CHROME_TRAVIS_CI' : 'CHROME_LOCAL';
} );

if ( shouldEnableBrowserStack() ) {
return browsers;
}

// If the BrowserStack is disabled, all browsers that start with a prefix "BrowserStack" should be filtered out.
// See: https://github.com/ckeditor/ckeditor5-dev/issues/358 and https://github.com/ckeditor/ckeditor5-dev/issues/402.
return browsers.filter( browser => !browser.startsWith( 'BrowserStack' ) );
}

// Formats name of the build for BrowserStack. It merges repository name and current timestamp.
// If env variable `TRAVIS_REPO_SLUG` is not available, the function returns `undefined`.
//
// @returns {String|undefined}
function getBuildName() {
const repoSlug = process.env.TRAVIS_REPO_SLUG;

if ( !repoSlug ) {
return;
}

const repositoryName = repoSlug.split( '/' )[ 1 ].replace( /-/g, '_' );
const date = new Date().getTime();

return `${ repositoryName } ${ date }`;
}

function shouldEnableBrowserStack() {
if ( !process.env.BROWSER_STACK_USERNAME ) {
return false;
}

if ( !process.env.BROWSER_STACK_ACCESS_KEY ) {
return false;
}
return options.browsers.map( browser => {
if ( browser !== 'Chrome' ) {
return browser;
}

// If the repository slugs are different, the pull request comes from the community (forked repository).
// For such builds, BrowserStack will be disabled. Read more: https://github.com/ckeditor/ckeditor5-dev/issues/358.
return ( process.env.TRAVIS_EVENT_TYPE !== 'pull_request' || process.env.TRAVIS_PULL_REQUEST_SLUG === process.env.TRAVIS_REPO_SLUG );
return process.env.TRAVIS ? 'CHROME_TRAVIS_CI' : 'CHROME_LOCAL';
} );
}
3 changes: 1 addition & 2 deletions packages/ckeditor5-dev-tests/lib/utils/automated-tests/parsearguments.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ module.exports = function parseArguments( args ) {
coverage: false,
verbose: false,
'source-map': false,
server: false,
browserStack: false
server: false
}
};
const options = minimist( args, minimistConfig );
Expand Down
1 change: 0 additions & 1 deletion packages/ckeditor5-dev-tests/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
"glob": "^7.1.3",
"istanbul-instrumenter-loader": "^3.0.0",
"karma": "^3.0.0",
"karma-browserstack-launcher": "^1.3.0",
"karma-chai": "^0.1.0",
"karma-chrome-launcher": "^2.2.0",
"karma-coverage": "^1.1.1",
Expand Down
77 changes: 1 addition & 76 deletions packages/ckeditor5-dev-tests/tests/utils/automated-tests/getkarmaconfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const { expect } = require( 'chai' );
const sinon = require( 'sinon' );
const path = require( 'path' );

describe( 'getKarmaConfig', () => {
describe( 'getKarmaConfig()', () => {
let getKarmaConfig, sandbox;
const originalEnv = process.env;

Expand Down Expand Up @@ -71,79 +71,4 @@ describe( 'getKarmaConfig', () => {
expect( karmaConfig ).to.have.own.property( 'browsers' );
expect( karmaConfig ).to.have.own.property( 'singleRun', true );
} );

describe( 'BrowserStack integration', () => {
beforeEach( () => {
process.env.BROWSER_STACK_USERNAME = 'username';
process.env.BROWSER_STACK_ACCESS_KEY = 'access-key';
} );

// BROWSER_STACK_USERNAME=username BROWSER_STACK_ACCESS_KEY=access-key yarn run test --files=autoformat
it( 'should be enabled when tests were called on a dev machine', () => {
// Removes the Travis variables. It protects tests on Travis where the env variables are set by default.
process.env.TRAVIS_EVENT_TYPE = null;
process.env.TRAVIS_REPO_SLUG = null;
process.env.TRAVIS_PULL_REQUEST_SLUG = null;

const karmaConfig = getKarmaConfig( { reporter: 'mocha', globPatterns: {}, browsers: [ 'BrowserStack_Edge' ] } );

expect( karmaConfig.browserStack ).to.not.be.a( 'undefined' );
expect( karmaConfig.browsers ).to.deep.equal( [
'BrowserStack_Edge'
] );
} );

// A team member made a commit. Travis should use BrowserStack.
it( 'should be enabled for commit build on Travis', () => {
process.env.TRAVIS = true;
process.env.TRAVIS_EVENT_TYPE = 'push';

const karmaConfig = getKarmaConfig( { reporter: 'mocha', globPatterns: {}, browsers: [ 'BrowserStack_Edge' ] } );

expect( karmaConfig.browserStack ).to.not.be.a( 'undefined' );
expect( karmaConfig.browsers ).to.deep.equal( [
'BrowserStack_Edge'
] );
} );

// A team member made a pull request. Travis should use BrowserStack.
it( 'should be enabled for pull request build on Travis', () => {
process.env.TRAVIS = true;
process.env.TRAVIS_EVENT_TYPE = 'pull_request';
process.env.TRAVIS_PULL_REQUEST_SLUG = 'ckeditor/ckeditor-foo';
process.env.TRAVIS_REPO_SLUG = 'ckeditor/ckeditor-foo';

const karmaConfig = getKarmaConfig( { reporter: 'mocha', globPatterns: {}, browsers: [ 'BrowserStack_Edge' ] } );

expect( karmaConfig.browserStack ).to.not.be.a( 'undefined' );
expect( karmaConfig.browsers ).to.deep.equal( [
'BrowserStack_Edge'
] );
} );

// A community member made a pull request. Travis should not use BrowserStack.
it( 'should be disabled for pull request build on Travis that comes from community', () => {
process.env.TRAVIS = true;
process.env.TRAVIS_EVENT_TYPE = 'pull_request';
process.env.TRAVIS_PULL_REQUEST_SLUG = 'ckeditor-forked/ckeditor-foo';
process.env.TRAVIS_REPO_SLUG = 'ckeditor/ckeditor-foo';

// Encrypted environment variables are not available to pull requests from forks due to
// the security risk of exposing such information to unknown code.
delete process.env.BROWSER_STACK_USERNAME;
delete process.env.BROWSER_STACK_ACCESS_KEY;

const karmaConfig = getKarmaConfig( {
reporter: 'mocha',
globPatterns: {},
browsers: [ 'BrowserStack_Edge', 'Firefox', 'Chrome', 'BrowserStack_Safari' ]
} );

expect( karmaConfig.browserStack ).to.be.a( 'undefined' );
expect( karmaConfig.browsers ).to.deep.equal( [
'Firefox',
'CHROME_TRAVIS_CI'
] );
} );
} );
} );