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

t/1: Introduce tests and Bender.js #11

Merged
merged 32 commits into from Feb 12, 2015
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
118a4eb
Added npm devDependency for bender with mocha and chai.
fredck Jan 9, 2015
ffe17c5
Initialized the bender configuration file.
fredck Jan 9, 2015
e7e9834
Introduced the "ckeditor" application in bender and the CKEditor 5 pl…
fredck Jan 16, 2015
09fa91e
Added code coverage support for bender.
fredck Jan 20, 2015
c9ad4d0
Added basic tests for AMD.
fredck Jan 16, 2015
5aa7d5a
Made it in a way that "ckeditor-core" keeps its original definition s…
fredck Jan 20, 2015
dd79ce5
Removed code that should (hopefully) be deprecated as scripts always …
fredck Jan 20, 2015
2946304
Added a "dev" flag to CKEDITOR.
fredck Jan 20, 2015
47cde86
Added tests for CKEDITOR.
fredck Jan 20, 2015
87f8200
Changed the ckeditor plugin for bender to user bender.defer().
fredck Jan 21, 2015
19e29d0
Reviewed the tests titles and made better use of "describe" to better…
fredck Jan 21, 2015
6488116
Standardized the equality checks to `to.equal()`.
fredck Jan 21, 2015
0c63b0a
Fixed a wrong object check in tests.
fredck Jan 21, 2015
ebcbd4e
Minor white-space cleanup.
fredck Jan 21, 2015
758b958
Fixed an override reversion that would not take place in case of test…
fredck Jan 21, 2015
26f6c7c
Introduced bender.amd.require() - a tool to easily require CKEditor m…
fredck Jan 22, 2015
41ec3ce
Reviewed tests to use bender.amd.require().
fredck Jan 22, 2015
70373f7
Avoided a test dependency on utils.isObject(), which is not in use an…
fredck Jan 22, 2015
1137ff7
Added support for describe.skip().
fredck Jan 22, 2015
a7f512d
Revert "Added support for describe.skip()."
fredck Jan 22, 2015
907ef34
Revert "Reviewed tests to use bender.amd.require()."
fredck Jan 22, 2015
af63433
Revert "Introduced bender.amd.require() - a tool to easily require CK…
fredck Jan 22, 2015
a8afa0a
Re-introduced bender.amd.require() - a tool to easily require CKEdito…
fredck Jan 22, 2015
338f468
Re-reviewed tests to use bender.amd.require().
fredck Jan 22, 2015
c15dbcd
Moved the bender.amd.require() call out of describe().
fredck Jan 22, 2015
938c8d9
Removing third-party libs from code coverage.
fredck Jan 22, 2015
6bf8aa1
Passing code coverage through untestable code.
fredck Jan 22, 2015
61c5f9c
Upgraded to Bender 0.2.
fredck Feb 3, 2015
0de6f6b
Corrected the plugin name.
fredck Feb 3, 2015
a60cb57
Added support for Sinon.
fredck Feb 5, 2015
a530d20
Exclude code coverage reports from the repository.
gregpabian Feb 12, 2015
0968169
Minor typo fixes.
gregpabian Feb 12, 2015
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 46 additions & 0 deletions bender.js
@@ -0,0 +1,46 @@
/* jshint browser: false, node: true */

'use strict';

var config = {
plugins: [
// Uncomment to enable code coverage.
// 'benderjs-coverage',

'benderjs-mocha',
'benderjs-chai',
'dev/bender/plugins/ckeditor5'
],

framework: 'mocha',

applications: {
ckeditor: {
path: '.',
files: [
'node_modules/requirejs/require.js',
'ckeditor.js'
]
}
},

tests: {
all: {
applications: [ 'ckeditor' ],
paths: [
'tests/**',
'node_modules/ckeditor5-*/tests/**'
]
}
},

coverage: {
paths: [
'ckeditor.js',
'src/**/*.js',
'node_modules/ckeditor5-*/src/**/*.js'
]
}
};

module.exports = config;
15 changes: 3 additions & 12 deletions ckeditor.js
Expand Up @@ -3,7 +3,7 @@
* For licensing, see LICENSE.md.
*/

/* global requirejs, define, require, window, document, location */
/* global requirejs, define, require, window, document */

'use strict';

Expand Down Expand Up @@ -73,10 +73,9 @@

// Define a new "ckeditor" module, which overrides the core one with the above and the dev stuff.
define( 'ckeditor', [ 'ckeditor-core', 'ckeditor-dev', 'utils' ], function( core, dev, utils ) {
utils.extend( core, root.CKEDITOR, ( dev || {} ) );
root.CKEDITOR = core;
root.CKEDITOR = utils.extend( {}, core, root.CKEDITOR, ( dev || {} ) );

return core;
return root.CKEDITOR;
} );

function getBasePath() {
Expand All @@ -99,14 +98,6 @@
}
} );

if ( path.indexOf( ':/' ) == -1 && path.slice( 0, 2 ) != '//' ) {
if ( path[ 0 ] == '/' ) {
path = location.href.match( /^.*?:\/\/[^\/]*/ )[ 0 ] + path;
} else {
path = location.href.match( /^[^\?]*\/(?:)/ )[ 0 ] + path;
}
}

return path;
}
} )( window );
20 changes: 20 additions & 0 deletions dev/bender/plugins/ckeditor5/lib/index.js
@@ -0,0 +1,20 @@
/**
* @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* jshint node: true */

'use strict';

var path = require( 'path' );
var files = [
path.join( __dirname, '../static/extensions.js' )
];

module.exports = {
name: 'bender-framework-ckeditor5',
Copy link
Contributor

Choose a reason for hiding this comment

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

framework is no longer needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with 0de6f6b.


files: files,
include: files
};
5 changes: 5 additions & 0 deletions dev/bender/plugins/ckeditor5/package.json
@@ -0,0 +1,5 @@
{
"name": "benderjs-ckeditor5",
"description": "CKEditor 5 plugin for Bender.js",
"main": "lib/index.js"
}
16 changes: 16 additions & 0 deletions dev/bender/plugins/ckeditor5/static/extensions.js
@@ -0,0 +1,16 @@
/**
* @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* globals bender, CKEDITOR */

'use strict';

( function() {
// Make Bender wait to start running tests.
var done = bender.defer();

// Wait for the "ckeditor" module to be ready to start testing.
CKEDITOR.require( [ 'ckeditor' ], done );
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need this one now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It may be needed if bender.amd.require() is not used (e.g., the amd test). I think it is better to keep it just in case and it should not be a big deal anyway.

} )();
4 changes: 4 additions & 0 deletions package.json
Expand Up @@ -16,6 +16,10 @@
"del": "~1.1",
"ncp": "~1.0",
"replace": "~0.3.0",
"benderjs": "~0.1",
"benderjs-mocha": "~0.1",
"benderjs-chai": "~0.1.1",
"benderjs-coverage": "~0.1.1",
"grunt": "~0",
"grunt-jscs": "~1",
"grunt-contrib-jshint": "~0",
Expand Down
8 changes: 8 additions & 0 deletions src/ckeditor-dev.js
Expand Up @@ -9,6 +9,14 @@

define( 'ckeditor-dev', function() {
return {
/**
* A flag specifying whether CKEditor is running in development mode (original source code).
*
* This property is not defined in production (compiled, build code).
*/
isDev: true,

// Documented in ckeditor-core/ckeditor.
getPluginPath: function( name ) {
return this.basePath + 'node_modules/ckeditor-plugin-' + name + '/src/';
}
Expand Down
30 changes: 30 additions & 0 deletions tests/amd/amd.js
@@ -0,0 +1,30 @@
/**
* @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* globals describe, it, expect, CKEDITOR */
Copy link
Contributor

Choose a reason for hiding this comment

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

We're gonna have describe, it and expect in every test, maybe we can "globalize" them in a single place, like .jshintrc file in the tests/ directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i tried this already but it doesn't work in the way we're using jshint right now (by pointing to a specific configuration file).

If we would like to do it it looks like we would have to bloat (further) the root by adding . jshintrc there (and at this point .jscs) as well.

Copy link
Member

Choose a reason for hiding this comment

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

Heh... I've been already thinking about this too. I think that the easiest (thus, doable) solutions are:

  • Define little in the global configs and use comments extensively. The thing is that the global config of the ckeditor5-core module should for example contain CKEDITOR because it should be available in most of the files, but global config of the ckeditor5 module does not have to, because many files in this repository are related to grunt tasks. The rest will need to be defined in comments, which is not that bad, because you only need to do this when you start using some variable.
  • Define a lot in the global configs. We can add most of the stuff that can be available somewhere in tests or dev code to the global config and we won't need to add so many comments. This makes the check less precise, but it's still good.

In CKEditor 4 we use now the second approach and it's ok. For instance, you never use it or describe in the dev code, so you don't do mistakes related to them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gonna have an issue for this once this PR gets merged.

Copy link
Member

Choose a reason for hiding this comment

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

I reported #13.


'use strict';

CKEDITOR.define( 'testModule', [ 'ckeditor', 'utils' ], function( ckeditor, utils ) {
return {
test: utils.isObject( ckeditor )
};
} );

describe( 'CKEDITOR.require()', function() {
it( 'should work with a defined module', function( done ) {
CKEDITOR.require( [ 'testModule' ], function( testModule ) {
expect( testModule ).to.have.property( 'test' ).to.be.true();
done();
} );
} );

it( 'should work with a core module', function( done ) {
CKEDITOR.require( [ 'utils' ], function( utils ) {
expect( utils ).to.be.an( 'object' );
done();
} );
} );
} );
72 changes: 72 additions & 0 deletions tests/ckeditor/basepath.js
@@ -0,0 +1,72 @@
/**
* @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* globals beforeEach, describe, it, expect, CKEDITOR, window, document */

'use strict';

beforeEach( function() {
// Ensure that no CKEDITOR_BASEPATH global is available.
delete window.CKEDITOR_BASEPATH;

// Remove all script elements from the document.
removeScripts();
} );

describe( 'basePath', function() {
it( 'should work with script tags', function( done ) {
CKEDITOR.require( [ 'ckeditor' ], function( CKEDITOR ) {
addScript( 'http://bar.com/ckeditor/ckeditor.js' );
expect( CKEDITOR._getBasePath() ).to.equal( 'http://bar.com/ckeditor/' );
done();
} );
} );

it( 'should work with the CKEDITOR_BASEPATH global', function( done ) {
CKEDITOR.require( [ 'ckeditor' ], function( CKEDITOR ) {
window.CKEDITOR_BASEPATH = 'http://foo.com/ckeditor/';
expect( CKEDITOR._getBasePath() ).to.equal( 'http://foo.com/ckeditor/' );
done();
} );
} );
} );

describe( 'This browser', function() {
it( 'should not keep script URLs absolute or relative', function( done ) {
// Browsers should convert absolute and relative URLs to full URLs.
// If this test fails in any browser, _getBasePath() must be reviewed to deal with such case (v4 does it).

test( '/absolute/url/ckeditor.js' );
test( '../relative/url/ckeditor.js' );

done();

function test( url ) {
removeScripts();

var script = addScript( url );

// Test if the src now contains '://'.
expect( /:\/\//.test( script.src ) ).to.be.true();
}
} );
} );

function addScript( url ) {
var script = document.createElement( 'script' );
script.src = url;
document.head.appendChild( script );

return script;
}

function removeScripts() {
var scripts = [].slice.call( document.getElementsByTagName( 'script' ) );
var script;

while ( ( script = scripts.shift() ) ) {
script.parentNode.removeChild( script );
}
}
43 changes: 43 additions & 0 deletions tests/ckeditor/ckeditor.js
@@ -0,0 +1,43 @@
/**
* @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* globals describe, it, expect, CKEDITOR */

'use strict';

describe( 'getPluginPath()', function() {
it( 'should return a proper path', function( done ) {
CKEDITOR.require( [ 'ckeditor' ], function( CKEDITOR ) {
var basePath = CKEDITOR.basePath;
var path = CKEDITOR.getPluginPath( 'test' );

if ( CKEDITOR.isDev ) {
expect( path ).to.equal( basePath + 'node_modules/ckeditor-plugin-test/src/' );
} else {
expect( path ).to.equal( basePath + 'plugins/test/' );
}
done();
} );
} );

it( '(the production version) should work even when in dev', function( done ) {
CKEDITOR.require( [ 'ckeditor', 'ckeditor-core' ], function( CKEDITOR, core ) {
// To be able to run this test on both dev and production code, we need to override getPluginPath with the
// core version of it and restore it after testing.
var originalGetPluginPath = CKEDITOR.getPluginPath;
CKEDITOR.getPluginPath = core.getPluginPath;

// This test is good for both the development and production codes.
var basePath = CKEDITOR.basePath;
var path = CKEDITOR.getPluginPath( 'test' );

expect( path ).to.equal( basePath + 'plugins/test/' );

CKEDITOR.getPluginPath = originalGetPluginPath;
Copy link
Member

Choose a reason for hiding this comment

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

It's not going to be executed if expect().equal() throws. I don't know how it works in Mocha+Chai, but in CKE4 you would need to handle teardown outside the test or in try-catch-finally :D.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch... I've just pushed a fix for it by simply reverting the override before the assertion.


done();
} );
} );
} );