Skip to content

Commit

Permalink
GH-832: Look at devDeps for restoring platforms (#835)
Browse files Browse the repository at this point in the history
* GH-832: Look at devDeps for restoring platforms

This also includes fixes for writing package.json with the correct
newlines, which should address the complaints raised in [1] and possibly
[2].

1. apache/cordova-cli#353
2. #694

* Ensure a failure to restore stops the build

This has been a recurring frustration over several years, and nobody can
seem to explain why we would want to silently ignore restore failures
for platforms and plugins rather than surfacing them and failing the
remainder of the build steps.
  • Loading branch information
dpogue authored Jun 3, 2020
1 parent 7a20be8 commit 909e7c7
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 198 deletions.
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"cordova-serve": "^3.0.0",
"dep-graph": "^1.1.0",
"detect-indent": "^6.0.0",
"detect-newline": "^3.1.0",
"elementtree": "^0.1.7",
"execa": "^3.2.0",
"fs-extra": "^8.1.0",
Expand All @@ -31,7 +32,9 @@
"init-package-json": "^1.10.3",
"md5-file": "^4.0.0",
"pify": "^4.0.1",
"semver": "^6.3.0"
"semver": "^6.3.0",
"stringify-package": "^1.0.0",
"write-file-atomic": "^3.0.3"
},
"devDependencies": {
"@cordova/eslint-config": "^2.0.0",
Expand Down
28 changes: 21 additions & 7 deletions spec/cordova/restore-util.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

const path = require('path');
const fs = require('fs-extra');
const rewire = require('rewire');
const { tmpDir: getTmpDir, testPlatform } = require('../helpers');
const projectTestHelpers = require('../project-test-helpers');

Expand All @@ -39,14 +38,15 @@ describe('cordova/restore-util', () => {
configXmlPath = getConfigXmlPath();
delete process.env.PWD;

restore = require('../../src/cordova/restore-util');

cordovaPlugin = require('../../src/cordova/plugin');
spyOn(cordovaPlugin, 'add')
.and.returnValue(Promise.resolve());

cordovaPlatform = jasmine.createSpy('cordovaPlatform')
cordovaPlatform = require('../../src/cordova/platform');
spyOn(cordovaPlatform, 'add')
.and.returnValue(Promise.resolve());
restore = rewire('../../src/cordova/restore-util');
restore.__set__({ cordovaPlatform });

setupBaseProject();
});
Expand Down Expand Up @@ -90,7 +90,17 @@ describe('cordova/restore-util', () => {
}

function expectPlatformAdded (platform) {
expect(cordovaPlatform).toHaveBeenCalledWith('add', platform, undefined);
const expectedOpts = {
platforms: jasmine.arrayContaining([
jasmine.stringMatching(platform)
])
};

expect(cordovaPlatform.add).toHaveBeenCalledWith(
jasmine.anything(), jasmine.any(String),
jasmine.arrayContaining([platform]),
jasmine.objectContaining(expectedOpts)
);
}

function expectPluginAdded (plugin) {
Expand Down Expand Up @@ -141,7 +151,9 @@ describe('cordova/restore-util', () => {
// No change to pkg.json.
const pkgJson = getPkgJson();
expect(pkgJson.cordova.platforms).toEqual([PLATFORM]);
expect(pkgJson.dependencies[platformPkgName(PLATFORM)]).toEqual(`git+${PLATFORM_URL}.git`);

const specs = Object.assign({}, pkgJson.dependencies, pkgJson.devDependencies);
expect(specs[platformPkgName(PLATFORM)]).toEqual(`git+${PLATFORM_URL}.git`);
});
});

Expand Down Expand Up @@ -177,8 +189,10 @@ describe('cordova/restore-util', () => {
// Expect both pkg.json and config.xml to each have both platforms in their arrays.
expect(getCfgEngineNames()).toEqual([PLATFORM_1, PLATFORM_2]);
expect(pkgJson.cordova.platforms).toEqual([PLATFORM_1, PLATFORM_2]);

// Platform specs from config.xml have been added to pkg.json.
expect(pkgJson.dependencies).toEqual({
const specs = Object.assign({}, pkgJson.dependencies, pkgJson.devDependencies);
expect(specs).toEqual({
[platformPkgName(PLATFORM_1)]: '7.0.0',
[platformPkgName(PLATFORM_2)]: '^5.0.3'
});
Expand Down
Loading

0 comments on commit 909e7c7

Please sign in to comment.