Merge pull request #135 from rollup/coverage

remove some unused code, add some additional tests

package.json

@@ -11,9 +11,9 @@
     "pretest": "npm run build",
     "test": "mocha",
     "pretest-coverage": "npm run build",
-    "test-coverage": "istanbul cover --report json node_modules/.bin/_mocha -- -u exports -R spec test/test.js",
+    "test-coverage": "rm -rf coverage/* && istanbul cover --report json node_modules/.bin/_mocha -- -u exports -R spec test/test.js",
-    "posttest-coverage": "remap-istanbul -i coverage/coverage-final.json -o coverage/coverage-remapped.json -b dist",
+    "posttest-coverage": "remap-istanbul -i coverage/coverage-final.json -o coverage/coverage-remapped.json -b dist && remap-istanbul -i coverage/coverage-final.json -o coverage/coverage-remapped.lcov -t lcovonly -b dist && remap-istanbul -i coverage/coverage-final.json -o coverage/coverage-remapped -t html -b dist",
-    "ci": "npm run test-coverage && codecov < coverage/coverage-remapped.json",
+    "ci": "npm run test-coverage && codecov < coverage/coverage-remapped.lcov",
     "build": "gobble build -f dist",
     "prepublish": "npm test",
     "lint": "eslint src"

src/Module.js

@@ -599,19 +599,6 @@ export default class Module {
 					magicString.remove( statement.start, statement.next );
 					return;
 				}
-
-				// skip `export var foo;` if foo is exported
-				if ( isEmptyExportedVarDeclaration( statement.node.declaration, this.exports, toExport ) ) {
-					magicString.remove( statement.start, statement.next );
-					return;
-				}
-			}
-
-			// skip empty var declarations for exported bindings
-			// (otherwise we're left with `exports.foo;`, which is useless)
-			if ( isEmptyExportedVarDeclaration( statement.node, this.exports, toExport ) ) {
-				magicString.remove( statement.start, statement.next );
-				return;
 			}
 
 			// split up/remove var declarations as necessary

src/Statement.js

@@ -474,24 +474,6 @@ export default class Statement {
 							magicString.overwrite( node.start, id.end, bundleExports[ name ], true );
 							id._skip = true;
 						}
-
-						// otherwise, we insert the `exports.foo = foo` after the declaration
-						else {
-							const exportInitialisers = node.declarations
-								.map( declarator => declarator.id.name )
-								.filter( name => !!bundleExports[ name ] )
-								.map( name => `\n${bundleExports[name]} = ${name};` )
-								.join( '' );
-
-							if ( exportInitialisers ) {
-								// TODO clean this up
-								try {
-									magicString.insert( node.end, exportInitialisers );
-								} catch ( err ) {
-									magicString.append( exportInitialisers );
-								}
-							}
-						}
 					}
 				}
 

test/function/cannot-import-self/_config.js

@@ -0,0 +1,8 @@
+var assert = require( 'assert' );
+
+module.exports = {
+	description: 'prevents a module importing itself',
+	error: function ( err ) {
+		assert.ok( /A module cannot import itself/.test( err.message ) );
+	}
+};

test/function/cannot-import-self/main.js

@@ -0,0 +1 @@
+import me from './main';

test/function/export-type-mismatch-b/_config.js

@@ -0,0 +1,11 @@
+var assert = require( 'assert' );
+
+module.exports = {
+	description: 'export type must be auto, default, named or none',
+	bundleOptions: {
+		exports: 'blah'
+	},
+	generateError: function ( err ) {
+		assert.ok( /options\.exports must be 'default', 'named', 'none', 'auto', or left unspecified/.test( err.message ) );
+	}
+};

test/function/export-type-mismatch-b/main.js

@@ -0,0 +1 @@
+export default 42;

test/function/export-type-mismatch-c/_config.js

@@ -0,0 +1,11 @@
+var assert = require( 'assert' );
+
+module.exports = {
+	description: 'cannot have named exports if explicit export type is default',
+	bundleOptions: {
+		exports: 'none'
+	},
+	generateError: function ( err ) {
+		assert.ok( /'none' was specified for options\.exports/.test( err.message ) );
+	}
+};

test/function/export-type-mismatch-c/main.js

@@ -0,0 +1 @@
+export default 42;

test/function/export-type-mismatch/_config.js

@@ -0,0 +1,11 @@
+var assert = require( 'assert' );
+
+module.exports = {
+	description: 'cannot have named exports if explicit export type is default',
+	bundleOptions: {
+		exports: 'default'
+	},
+	generateError: function ( err ) {
+		assert.ok( /'default' was specified for options\.exports/.test( err.message ) );
+	}
+};

test/function/export-type-mismatch/main.js

@@ -0,0 +1 @@
+export var foo = 42;

test/function/property-keys-not-renamed/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'does not rename property keys'
+};

test/function/property-keys-not-renamed/main.js

@@ -0,0 +1,7 @@
+import one from './one';
+import two from './two';
+import three from './three';
+
+assert.equal( one(), 'one' );
+assert.equal( two(), 'two' );
+assert.equal( three(), 'three' );

test/function/property-keys-not-renamed/one.js

@@ -0,0 +1,11 @@
+var obj = {
+	foo: foo
+};
+
+function foo () {
+	return 'one';
+}
+
+export default function () {
+	return obj.foo();
+}

test/function/property-keys-not-renamed/three.js

@@ -0,0 +1,11 @@
+var obj = {
+	foo: foo
+};
+
+function foo () {
+	return 'three';
+}
+
+export default function () {
+	return obj.foo();
+}

test/function/property-keys-not-renamed/two.js

@@ -0,0 +1,11 @@
+var obj = {
+	foo: foo
+};
+
+function foo () {
+	return 'two';
+}
+
+export default function () {
+	return obj.foo();
+}

test/test.js

@@ -45,6 +45,36 @@ describe( 'rollup', function () {
 		it( 'has a rollup method', function () {
 			assert.equal( typeof rollup.rollup, 'function' );
 		});
+
+		it( 'fails without options or options.entry', function () {
+			assert.throws( function () {
+				rollup.rollup();
+			}, /must supply options\.entry/ );
+
+			assert.throws( function () {
+				rollup.rollup({});
+			}, /must supply options\.entry/ );
+		});
+	});
+
+	describe( 'bundle.write()', function () {
+		it( 'fails without options or options.dest', function () {
+			return rollup.rollup({
+				entry: 'x',
+				resolveId: function () { return 'test'; },
+				load: function () {
+					return '// empty';
+				}
+			}).then( function ( bundle ) {
+				assert.throws( function () {
+					bundle.write();
+				}, /must supply options\.dest/ );
+
+				assert.throws( function () {
+					bundle.write({});
+				}, /must supply options\.dest/ );
+			});
+		});
 	});
 
 	describe( 'function', function () {
@@ -80,22 +110,22 @@ describe( 'rollup', function () {
 								format: 'cjs'
 							}));
 
-							if ( config.error ) {
+							if ( config.generateError ) {
 								unintendedError = new Error( 'Expected an error while generating output' );
 							}
 						} catch ( err ) {
-							if ( config.error ) {
+							if ( config.generateError ) {
-								config.error( err );
+								config.generateError( err );
 							} else {
 								unintendedError = err;
 							}
 						}
 
 						if ( unintendedError ) throw unintendedError;
+						if ( config.error || config.generateError ) return;
 
 						var code;
 
-						try {
 						if ( config.babel ) {
 							code = babel.transform( result.code, {
 								blacklist: [ 'es6.modules' ],
@@ -130,13 +160,6 @@ describe( 'rollup', function () {
 
 						if ( config.exports ) config.exports( module.exports );
 						if ( config.bundle ) config.bundle( bundle );
-						} catch ( err ) {
-							if ( config.error ) {
-								config.error( err );
-							} else {
-								unintendedError = err;
-							}
-						}
 
 						if ( config.show || unintendedError ) {
 							console.log( code + '\n\n\n' );