From 1aacab5ccd6e1ca85c964ef4e45b4f6dc80c71e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Raphael=20von=20der=20Gr=C3=BCn?= Date: Wed, 30 May 2018 10:57:40 +0200 Subject: [PATCH 1/3] spec fix error messages for toExist matcher --- spec/helpers.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/helpers.js b/spec/helpers.js index daf6722e..b15486c5 100644 --- a/spec/helpers.js +++ b/spec/helpers.js @@ -85,9 +85,9 @@ beforeEach(function () { result.pass = fs.existsSync(testPath); if (result.pass) { - result.message = 'Expected file ' + testPath + ' to exist.'; - } else { result.message = 'Expected file ' + testPath + ' to not exist.'; + } else { + result.message = 'Expected file ' + testPath + ' to exist.'; } return result; From f4b752d0224f76da8c9204412c8d2af590664994 Mon Sep 17 00:00:00 2001 From: "Christopher J. Brody" Date: Wed, 30 May 2018 11:08:51 -0400 Subject: [PATCH 2/3] spec check more artifacts, with some refactoring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (some redundant test code removed) Co-authored-by: Christopher J. Brody Co-authored-by: Raphael von der Grün --- spec/create.spec.js | 153 ++++++++++++------ .../www/fixture-marker-page.html | 1 + .../www/subdir/sub-fixture-marker-page.html | 1 + 3 files changed, 108 insertions(+), 47 deletions(-) create mode 100644 spec/templates/config_in_www/www/fixture-marker-page.html create mode 100644 spec/templates/config_in_www/www/subdir/sub-fixture-marker-page.html diff --git a/spec/create.spec.js b/spec/create.spec.js index 68e924a3..022601c1 100644 --- a/spec/create.spec.js +++ b/spec/create.spec.js @@ -57,80 +57,130 @@ describe('cordova create checks for valid-identifier', function () { describe('create end-to-end', function () { - function checkProject () { + function checkProjectCommonArtifacts () { // Check if top level dirs exist. var dirs = ['hooks', 'platforms', 'plugins', 'www']; dirs.forEach(function (d) { expect(path.join(project, d)).toExist(); }); + // Check that README.md exists inside of hooks expect(path.join(project, 'hooks', 'README.md')).toExist(); - // Check if www files exist. + // Check that index.html exists inside of www expect(path.join(project, 'www', 'index.html')).toExist(); - // Check that config.xml was updated. - var configXml = new ConfigParser(path.join(project, 'config.xml')); - expect(configXml.packageName()).toEqual(appId); - - // TODO (kamrik): check somehow that we got the right config.xml from the fixture and not some place else. - // expect(configXml.name()).toEqual('TestBase'); - } + // Check if config.xml exists. + expect(path.join(project, 'config.xml')).toExist(); - function checkConfigXml () { - // Check if top level dirs exist. - var dirs = ['hooks', 'platforms', 'plugins', 'www']; - dirs.forEach(function (d) { - expect(path.join(project, d)).toExist(); - }); - expect(path.join(project, 'hooks', 'README.md')).toExist(); - - // index.js and template subdir folder should not exist (inner files should be copied to the project folder) + // index.html, index.js and template subdir folder + // should not exist in top level + // (inner files should be copied to the project top level folder) + expect(path.join(project, 'index.html')).not.toExist(); expect(path.join(project, 'index.js')).not.toExist(); expect(path.join(project, 'template')).not.toExist(); - // Check if www files exist. - expect(path.join(project, 'www', 'index.html')).toExist(); + // Check that .gitignore does not exist inside of www + expect(path.join(project, 'www', '.gitignore')).not.toExist(); + + // Check that .npmignore does not exist inside of www + expect(path.join(project, 'www', '.npmignore')).not.toExist(); + + // Check that config.xml does not exist inside of www + expect(path.join(project, 'www', 'config.xml')).not.toExist(); + + // Check that no package.json exists inside of www + expect(path.join(project, 'www', 'package.json')).not.toExist(); + + // Check that config.xml was updated. var configXml = new ConfigParser(path.join(project, 'config.xml')); expect(configXml.packageName()).toEqual(appId); expect(configXml.version()).toEqual('1.0.0'); + // Check that we got the right config.xml from the fixture and not some place else. + expect(configXml.name()).toEqual('TestBase'); + } - // Check that config.xml does not exist inside of www - expect(path.join(project, 'www', 'config.xml')).not.toExist(); + function checkProjectArtifactsWithConfigFromTemplate () { + checkProjectCommonArtifacts(); + + // Check that standard js artifact does not exist + expect(path.join(project, 'www', 'js')).not.toExist(); + expect(path.join(project, 'www', 'js', 'index.js')).not.toExist(); + + // [CB-12397] Check that .gitignore does not exist + expect(path.join(project, '.gitignore')).not.toExist(); + // [CB-12397] Check that .npmignore does not exist + expect(path.join(project, '.npmignore')).not.toExist(); // Check that we got no package.json expect(path.join(project, 'package.json')).not.toExist(); // Check that we got the right config.xml from the template and not stock + const configXml = new ConfigParser(path.join(project, 'config.xml')); expect(configXml.description()).toEqual('this is the correct config.xml'); } - function checkSubDir () { - // Check if top level dirs exist. - var dirs = ['hooks', 'platforms', 'plugins', 'www']; - dirs.forEach(function (d) { - expect(path.join(project, d)).toExist(); - }); - expect(path.join(project, 'hooks', 'README.md')).toExist(); + function checkProjectArtifactsWithNoPackageFromTemplate () { + checkProjectCommonArtifacts(); - // index.js and template subdir folder should not exist (inner files should be copied to the project folder) - expect(path.join(project, 'index.js')).not.toExist(); - expect(path.join(project, 'template')).not.toExist(); + // Check that standard js artifact does not exist + expect(path.join(project, 'www', 'js')).not.toExist(); + expect(path.join(project, 'www', 'js', 'index.js')).not.toExist(); + + // [CB-12397] Check that .gitignore does not exist + expect(path.join(project, '.gitignore')).not.toExist(); + // [CB-12397] Check that .npmignore does not exist + expect(path.join(project, '.npmignore')).not.toExist(); + + // Check that we got no package.json + expect(path.join(project, 'package.json')).not.toExist(); + } + + function checkProjectArtifactsWithPackageFromTemplate () { + checkProjectCommonArtifacts(); + + // Check that standard js artifact exists + expect(path.join(project, 'www', 'js')).toExist(); + expect(path.join(project, 'www', 'js', 'index.js')).toExist(); + + // Check if package.json exists. + expect(path.join(project, 'package.json')).toExist(); + + // [CB-12397] Check that .gitignore does not exist + expect(path.join(project, '.gitignore')).not.toExist(); + // [CB-12397] Check that .npmignore exists + expect(path.join(project, '.npmignore')).toExist(); + + // Check that we got package.json (the correct one) + var pkjson = requireFresh(path.join(project, 'package.json')); + // Pkjson.displayName should equal config's name. + expect(pkjson.displayName).toEqual('TestBase'); + } + + function checkProjectArtifactsWithPackageFromSubDir () { + checkProjectCommonArtifacts(); + + // Check that standard js artifact does not exist + expect(path.join(project, 'www', 'js')).not.toExist(); + expect(path.join(project, 'www', 'js', 'index.js')).not.toExist(); + + // [CB-12397] Check that .gitignore does not exist + expect(path.join(project, '.gitignore')).not.toExist(); + // [CB-12397] Check that .npmignore does not exist + expect(path.join(project, '.npmignore')).not.toExist(); // Check if config files exist. expect(path.join(project, 'www', 'index.html')).toExist(); - // Check that config.xml was updated. - var configXml = new ConfigParser(path.join(project, 'config.xml')); - expect(configXml.packageName()).toEqual(appId); - expect(configXml.version()).toEqual('1.0.0'); // Check that we got package.json (the correct one) var pkjson = requireFresh(path.join(project, 'package.json')); + // Pkjson.displayName should equal config's name. expect(pkjson.displayName).toEqual(appName); expect(pkjson.valid).toEqual('true'); // Check that we got the right config.xml + const configXml = new ConfigParser(path.join(project, 'config.xml')); expect(configXml.description()).toEqual('this is the correct config.xml'); } @@ -138,7 +188,7 @@ describe('create end-to-end', function () { // Create a real project with no template // use default cordova-app-hello-world app return create(project, appId, appName, {}, events) - .then(checkProject) + .then(checkProjectArtifactsWithPackageFromTemplate) .then(function () { var pkgJson = requireFresh(path.join(project, 'package.json')); // confirm default hello world app copies over package.json and it matched appId @@ -161,7 +211,7 @@ describe('create end-to-end', function () { expect(fetchSpy).toHaveBeenCalledTimes(1); expect(fetchSpy.calls.argsFor(0)[0]).toBe(config.lib.www.url); }) - .then(checkProject); + .then(checkProjectArtifactsWithPackageFromTemplate); }); it('should successfully run with NPM package', function () { @@ -179,7 +229,7 @@ describe('create end-to-end', function () { expect(fetchSpy).toHaveBeenCalledTimes(1); expect(fetchSpy.calls.argsFor(0)[0]).toBe(config.lib.www.url); }) - .then(checkProject); + .then(checkProjectArtifactsWithPackageFromTemplate); }); it('should successfully run with NPM package and explicitly fetch latest if no version is given', function () { @@ -198,7 +248,7 @@ describe('create end-to-end', function () { expect(fetchSpy).toHaveBeenCalledTimes(1); expect(fetchSpy.calls.argsFor(0)[0]).toBe(config.lib.www.url + '@latest'); }) - .then(checkProject); + .then(checkProjectArtifactsWithPackageFromTemplate); }); it('should successfully run with template not having a package.json at toplevel', function () { @@ -211,7 +261,7 @@ describe('create end-to-end', function () { } }; return create(project, appId, appName, config, events) - .then(checkProject) + .then(checkProjectArtifactsWithNoPackageFromTemplate) .then(function () { // Check that we got the right config.xml var configXml = new ConfigParser(path.join(project, 'config.xml')); @@ -229,7 +279,7 @@ describe('create end-to-end', function () { } }; return create(project, appId, appName, config, events) - .then(checkProject); + .then(checkProjectArtifactsWithNoPackageFromTemplate); }); it('should successfully run with template having package.json, and subdirectory, and no package.json in subdirectory', function () { @@ -242,7 +292,7 @@ describe('create end-to-end', function () { } }; return create(project, appId, appName, config, events) - .then(checkProject); + .then(checkProjectArtifactsWithNoPackageFromTemplate); }); it('should successfully run with template having package.json, and subdirectory, and package.json in subdirectory', function () { @@ -255,7 +305,7 @@ describe('create end-to-end', function () { } }; return create(project, appId, appName, config, events) - .then(checkSubDir); + .then(checkProjectArtifactsWithPackageFromSubDir); }); it('should successfully run config.xml in the www folder and move it outside', function () { @@ -268,7 +318,7 @@ describe('create end-to-end', function () { } }; return create(project, appId, appName, config, events) - .then(checkConfigXml); + .then(checkProjectArtifactsWithConfigFromTemplate); }); it('should successfully run with www folder as the template', function () { @@ -281,13 +331,20 @@ describe('create end-to-end', function () { } }; return create(project, appId, appName, config, events) - .then(checkConfigXml); + .then(checkProjectArtifactsWithConfigFromTemplate) + .then(() => { + // Additional check that we have the fixture www, + // not one from stock the app + expect(path.join(project, 'www', 'fixture-marker-page.html')).toExist(); + expect(path.join(project, 'www', 'subdir')).toExist(); + expect(path.join(project, 'www', 'subdir', 'sub-fixture-marker-page.html')).toExist(); + }); }); it('should successfully run with existing, empty destination', function () { shell.mkdir('-p', project); return create(project, appId, appName, {}, events) - .then(checkProject); + .then(checkProjectArtifactsWithPackageFromTemplate); }); describe('when --link-to is provided', function () { @@ -315,6 +372,7 @@ describe('create end-to-end', function () { // Check www/config exists expect(path.join(project, 'www', 'config.xml')).toExist(); + // Check www/config.xml was not updated. var configXml = new ConfigParser(path.join(project, 'www', 'config.xml')); expect(configXml.packageName()).toEqual('io.cordova.hellocordova'); @@ -323,6 +381,7 @@ describe('create end-to-end', function () { // Check that config.xml was copied to project/config.xml expect(path.join(project, 'config.xml')).toExist(); + configXml = new ConfigParser(path.join(project, 'config.xml')); expect(configXml.description()).toEqual('this is the correct config.xml'); // Check project/config.xml was updated. diff --git a/spec/templates/config_in_www/www/fixture-marker-page.html b/spec/templates/config_in_www/www/fixture-marker-page.html new file mode 100644 index 00000000..6a9821cc --- /dev/null +++ b/spec/templates/config_in_www/www/fixture-marker-page.html @@ -0,0 +1 @@ +

Fixture marker page

diff --git a/spec/templates/config_in_www/www/subdir/sub-fixture-marker-page.html b/spec/templates/config_in_www/www/subdir/sub-fixture-marker-page.html new file mode 100644 index 00000000..311ec826 --- /dev/null +++ b/spec/templates/config_in_www/www/subdir/sub-fixture-marker-page.html @@ -0,0 +1 @@ +

Sub-fixture marker page

From 754076d8adfb824d5041bfd84ecc5cac198f6529 Mon Sep 17 00:00:00 2001 From: "Christopher J. Brody" Date: Wed, 30 May 2018 17:08:45 -0400 Subject: [PATCH 3/3] use fs-extra instead of shelljs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Christopher J. Brody Co-authored-by: Raphael von der Grün --- index.js | 31 ++++++++++++++----------------- package.json | 2 +- spec/create.spec.js | 13 ++++++------- spec/helpers.js | 5 ++--- 4 files changed, 23 insertions(+), 28 deletions(-) diff --git a/index.js b/index.js index 61181cab..30b9fcf2 100644 --- a/index.js +++ b/index.js @@ -17,13 +17,13 @@ under the License. */ -var fs = require('fs'); +const fs = require('fs-extra'); + var os = require('os'); var path = require('path'); var Promise = require('q'); var isUrl = require('is-url'); -var shell = require('shelljs'); var requireFresh = require('import-fresh'); var validateIdentifier = require('valid-identifier'); @@ -232,11 +232,11 @@ module.exports = function (dir, optionalId, optionalName, cfg, extEvents) { copyIfNotExists(stockAssetPath('hooks'), path.join(dir, 'hooks')); var configXmlExists = projectConfig(dir); // moves config to root if in www if (!configXmlExists) { - shell.cp(stockAssetPath('config.xml'), path.join(dir, 'config.xml')); + fs.copySync(stockAssetPath('config.xml'), path.join(dir, 'config.xml')); } } catch (e) { if (!dirAlreadyExisted) { - shell.rm('-rf', dir); + fs.removeSync(dir); } if (process.platform.slice(0, 3) === 'win' && e.code === 'EPERM') { throw new CordovaError('Symlinks on Windows require Administrator privileges'); @@ -266,8 +266,8 @@ module.exports = function (dir, optionalId, optionalName, cfg, extEvents) { } // Create basic project structure. - shell.mkdir('-p', path.join(dir, 'platforms')); - shell.mkdir('-p', path.join(dir, 'plugins')); + fs.ensureDirSync(path.join(dir, 'platforms')); + fs.ensureDirSync(path.join(dir, 'plugins')); var configPath = path.join(dir, 'config.xml'); // only update config.xml if not a symlink @@ -290,8 +290,7 @@ module.exports = function (dir, optionalId, optionalName, cfg, extEvents) { */ function copyIfNotExists (src, dst) { if (!fs.existsSync(dst) && src) { - shell.mkdir(dst); - shell.cp('-R', path.join(src, '*'), dst); + fs.copySync(src, dst); } } @@ -310,7 +309,7 @@ function copyTemplateFiles (templateDir, projectDir, isSubDir) { // if template is a www dir if (path.basename(templateDir) === 'www') { copyPath = path.resolve(templateDir); - shell.cp('-R', copyPath, projectDir); + fs.copySync(copyPath, path.resolve(projectDir, 'www')); } else { var templateFiles = fs.readdirSync(templateDir); // Remove directories, and files that are unwanted @@ -321,10 +320,10 @@ function copyTemplateFiles (templateDir, projectDir, isSubDir) { }); } // Copy each template file after filter - for (var i = 0; i < templateFiles.length; i++) { - copyPath = path.resolve(templateDir, templateFiles[i]); - shell.cp('-R', copyPath, projectDir); - } + templateFiles.forEach(f => { + copyPath = path.resolve(templateDir, f); + fs.copySync(copyPath, path.resolve(projectDir, f)); + }); } } @@ -373,9 +372,7 @@ function linkFromTemplate (templateDir, projectDir) { var linkSrc, linkDst, linkFolders, copySrc, copyDst; function rmlinkSync (src, dst, type) { if (src && dst) { - if (fs.existsSync(dst)) { - shell.rm('-rf', dst); - } + fs.removeSync(dst); if (fs.existsSync(src)) { fs.symlinkSync(src, dst, type); } @@ -403,7 +400,7 @@ function linkFromTemplate (templateDir, projectDir) { // if template/www/config.xml then copy to project/config.xml copyDst = path.join(projectDir, 'config.xml'); if (!fs.existsSync(copyDst) && fs.existsSync(copySrc)) { - shell.cp(copySrc, projectDir); + fs.copySync(copySrc, copyDst); } } diff --git a/package.json b/package.json index 9ac95d1d..c4e7f37e 100644 --- a/package.json +++ b/package.json @@ -28,10 +28,10 @@ "cordova-app-hello-world": "^3.11.0", "cordova-common": "^2.2.0", "cordova-fetch": "^1.3.0", + "fs-extra": "^6.0.1", "import-fresh": "^2.0.0", "is-url": "^1.2.4", "q": "^1.5.1", - "shelljs": "^0.8.2", "valid-identifier": "0.0.1" }, "devDependencies": { diff --git a/spec/create.spec.js b/spec/create.spec.js index 022601c1..280dfbe7 100644 --- a/spec/create.spec.js +++ b/spec/create.spec.js @@ -17,10 +17,10 @@ under the License. */ -var fs = require('fs'); +const fs = require('fs-extra'); + var path = require('path'); -var shell = require('shelljs'); var requireFresh = require('import-fresh'); var create = require('..'); @@ -35,12 +35,11 @@ var project = path.join(tmpDir, appName); // Setup and teardown test dirs beforeEach(function () { - shell.rm('-rf', project); - shell.mkdir('-p', tmpDir); + fs.emptyDirSync(tmpDir); }); -afterEach(function () { +afterAll(function () { process.chdir(path.join(__dirname, '..')); // Needed to rm the dir on Windows. - shell.rm('-rf', tmpDir); + fs.removeSync(tmpDir); }); describe('cordova create checks for valid-identifier', function () { @@ -342,7 +341,7 @@ describe('create end-to-end', function () { }); it('should successfully run with existing, empty destination', function () { - shell.mkdir('-p', project); + fs.ensureDirSync(project); return create(project, appId, appName, {}, events) .then(checkProjectArtifactsWithPackageFromTemplate); }); diff --git a/spec/helpers.js b/spec/helpers.js index b15486c5..bc48248c 100644 --- a/spec/helpers.js +++ b/spec/helpers.js @@ -17,12 +17,11 @@ under the License. */ -const fs = require('fs'); +const fs = require('fs-extra'); const os = require('os'); const path = require('path'); const rewire = require('rewire'); -const shell = require('shelljs'); // Disable regular console output during tests const CordovaLogger = require('cordova-common').CordovaLogger; @@ -44,7 +43,7 @@ function createWithMockFetch (dir, id, name, cfg, events) { const fetchSpy = jasmine.createSpy('fetchSpy') .and.callFake(() => Promise.resolve(mockFetchDest)); - shell.cp('-R', templateDir, mockFetchDest); + fs.copySync(templateDir, mockFetchDest); return createWith({fetch: fetchSpy})(dir, id, name, cfg, events) .then(() => fetchSpy); }