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

refactor: do not infer project root from script location #1265

Merged
merged 5 commits into from
Jul 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion bin/lib/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ exports.create = function (project_path, config, options, events) {
? config.name().replace(/[^\w.]/g, '_') : 'CordovaExample';

var safe_activity_name = config.android_activityName() || options.activityName || 'MainActivity';
var target_api = check_reqs.get_target();
var target_api = check_reqs.get_target(project_path);

// Make the package conform to Java package types
return exports.validatePackageName(package_name)
Expand Down
4 changes: 2 additions & 2 deletions bin/templates/cordova/Api.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ function setupEvents (externalEventEmitter) {
class Api {
constructor (platform, platformRootDir, events) {
this.platform = PLATFORM;
this.root = path.resolve(__dirname, '..');
this.root = platformRootDir;

setupEvents(events);

Expand Down Expand Up @@ -313,7 +313,7 @@ class Api {
* objects for current platform.
*/
requirements () {
return require('./lib/check_reqs').check_all();
return require('./lib/check_reqs').check_all(this.root);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions bin/templates/cordova/lib/builders/ProjectBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ function findOutputFiles (bundleType, buildType, { arch } = {}) {

class ProjectBuilder {
constructor (rootDirectory) {
this.root = rootDirectory || path.resolve(__dirname, '../../..');
this.root = rootDirectory;
this.apkDir = path.join(this.root, 'app', 'build', 'outputs', 'apk');
this.aabDir = path.join(this.root, 'app', 'build', 'outputs', 'bundle');
}
Expand Down Expand Up @@ -310,7 +310,7 @@ class ProjectBuilder {
if (error.toString().includes('failed to find target with hash string')) {
// Add hint from check_android_target to error message
try {
await check_reqs.check_android_target();
await check_reqs.check_android_target(this.root);
} catch (checkAndroidTargetError) {
error.message += '\n' + checkAndroidTargetError.message;
}
Expand Down
27 changes: 17 additions & 10 deletions bin/templates/cordova/lib/check_reqs.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ var path = require('path');
var fs = require('fs-extra');
const { forgivingWhichSync, isWindows, isDarwin } = require('./utils');
const java = require('./env/java');
var REPO_ROOT = path.join(__dirname, '..', '..', '..', '..');
const { CordovaError, ConfigParser, events } = require('cordova-common');
var android_sdk = require('./android_sdk');
const { SDK_VERSION } = require('./gradle-config-defaults');
Expand All @@ -32,10 +31,11 @@ const { SDK_VERSION } = require('./gradle-config-defaults');
Object.assign(module.exports, { isWindows, isDarwin });

/**
* @param {string} projectRoot
* @returns {string} The android target in format "android-${target}"
*/
module.exports.get_target = function () {
const userTargetSdkVersion = getUserTargetSdkVersion();
module.exports.get_target = function (projectRoot) {
const userTargetSdkVersion = getUserTargetSdkVersion(projectRoot);

if (userTargetSdkVersion && userTargetSdkVersion < SDK_VERSION) {
events.emit('warn', `android-targetSdkVersion should be greater than or equal to ${SDK_VERSION}.`);
Expand All @@ -44,10 +44,16 @@ module.exports.get_target = function () {
return `android-${Math.max(userTargetSdkVersion, SDK_VERSION)}`;
};

/** @returns {number} target sdk or 0 if undefined */
function getUserTargetSdkVersion () {
/**
* @param {string} projectRoot
* @returns {number} target sdk or 0 if undefined
*/
function getUserTargetSdkVersion (projectRoot) {
// If the repo config.xml file exists, find the desired targetSdkVersion.
const configFile = path.join(REPO_ROOT, 'config.xml');
// We need to use the cordova project's config.xml here, since the platform
// project's config.xml does not yet have the user's preferences when this
// function is called during `Api.createPlatform`.
const configFile = path.join(projectRoot, '../../config.xml');
if (!fs.existsSync(configFile)) return 0;

const configParser = new ConfigParser(configFile);
Expand Down Expand Up @@ -235,13 +241,13 @@ module.exports.check_android = function () {
});
};

module.exports.check_android_target = function () {
module.exports.check_android_target = function (projectRoot) {
// valid_target can look like:
// android-19
// android-L
// Google Inc.:Google APIs:20
// Google Inc.:Glass Development Kit Preview:20
var desired_api_level = module.exports.get_target();
var desired_api_level = module.exports.get_target(projectRoot);
return android_sdk.list_targets().then(function (targets) {
if (targets.indexOf(desired_api_level) >= 0) {
return targets;
Expand Down Expand Up @@ -286,9 +292,10 @@ var Requirement = function (id, name, version, installed) {
* Methods that runs all checks one by one and returns a result of checks
* as an array of Requirement objects. This method intended to be used by cordova-lib check_reqs method
*
* @param {string} projectRoot
* @return Promise<Requirement[]> Array of requirements. Due to implementation, promise is always fulfilled.
*/
module.exports.check_all = function () {
module.exports.check_all = function (projectRoot) {
var requirements = [
new Requirement('java', 'Java JDK'),
new Requirement('androidSdk', 'Android SDK'),
Expand All @@ -299,7 +306,7 @@ module.exports.check_all = function () {
var checkFns = [
this.check_java,
this.check_android,
this.check_android_target,
this.check_android_target.bind(this, projectRoot),
this.check_gradle
];

Expand Down
6 changes: 5 additions & 1 deletion bin/templates/cordova/lib/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var emulator = require('./emulator');
const target = require('./target');
const build = require('./build');
const PackageType = require('./PackageType');
const AndroidManifest = require('./AndroidManifest');
const { CordovaError, events } = require('cordova-common');

/**
Expand Down Expand Up @@ -75,5 +76,8 @@ module.exports.run = async function (runOptions = {}) {
if (resolvedTarget.type === 'emulator') {
await emulator.wait_for_boot(resolvedTarget.id);
}
return target.install(resolvedTarget, buildResults);

const manifest = new AndroidManifest(this.locations.manifest);

return target.install(resolvedTarget, { manifest, buildResults });
};
5 changes: 1 addition & 4 deletions bin/templates/cordova/lib/target.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@
under the License.
*/

const path = require('path');
const { inspect } = require('util');
const execa = require('execa');
const Adb = require('./Adb');
const build = require('./build');
const emulator = require('./emulator');
const AndroidManifest = require('./AndroidManifest');
const { compareBy } = require('./utils');
const { retryPromise } = require('./retry');
const { events, CordovaError } = require('cordova-common');
Expand Down Expand Up @@ -129,9 +127,8 @@ exports.resolve = async (spec, buildResults) => {
};
};

exports.install = async function ({ id: target, arch, type }, buildResults) {
exports.install = async function ({ id: target, arch, type }, { manifest, buildResults }) {
const apk_path = build.findBestApkForArchitecture(buildResults, arch);
const manifest = new AndroidManifest(path.join(__dirname, '../../app/src/main/AndroidManifest.xml'));
const pkgName = manifest.getPackageId();
const launchName = pkgName + '/.' + manifest.getActivity().getName();

Expand Down
74 changes: 74 additions & 0 deletions spec/e2e/e2e.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
*/

const os = require('os');
const fs = require('fs-extra');
const path = require('path');
const { EventEmitter } = require('events');
const { ConfigParser, PluginInfoProvider } = require('cordova-common');
const Api = require('../../bin/templates/cordova/Api');

function makeTempDir () {
const tmpDirTemplate = path.join(os.tmpdir(), 'cordova-android-test-');
return fs.realpathSync(fs.mkdtempSync(tmpDirTemplate));
}

async function makeProject (projectPath) {
const configXmlPath = path.join(__dirname, '../../bin/templates/project/res/xml/config.xml');
const config = new ConfigParser(configXmlPath);
config.setPackageName('io.cordova.testapp');
config.setName('TestApp');

const noopEvents = new EventEmitter();

return Api.createPlatform(projectPath, config, {}, noopEvents);
}

describe('E2E', function () {
let tmpDir, projectPath, api;
beforeEach(async () => {
tmpDir = makeTempDir();

projectPath = path.join(tmpDir, 'project');
api = await makeProject(projectPath);
});
afterEach(() => {
fs.removeSync(tmpDir);
});

it('loads the API from a project directory', async () => {
// Allow test project to find the `cordova-android` module
fs.ensureSymlinkSync(
path.join(__dirname, '../..'),
path.join(tmpDir, 'node_modules/cordova-android'),
'junction'
);

expect(() => {
require(path.join(projectPath, 'cordova/Api.js'));
}).not.toThrow();
});

it('adds a plugin with framework', async () => {
const fakePluginPath = path.join(__dirname, 'fixtures/cordova-plugin-fake');
const pluginInfo = new PluginInfoProvider().get(fakePluginPath);

await expectAsync(api.addPlugin(pluginInfo)).toBeResolved();
});
});
72 changes: 0 additions & 72 deletions spec/e2e/plugin.spec.js

This file was deleted.

8 changes: 3 additions & 5 deletions spec/unit/builders/ProjectBuilder.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,8 @@ describe('ProjectBuilder', () => {
expect(builder.root).toBe(rootDir);
});

it('should set the project directory to three folders up', () => {
ProjectBuilder.__set__('__dirname', 'projecttest/platforms/android/app');
builder = new ProjectBuilder();
expect(builder.root).toMatch(/projecttest$/);
it('should throw if project directory is missing', () => {
expect(() => new ProjectBuilder()).toThrowError(TypeError);
});
});

Expand Down Expand Up @@ -224,7 +222,7 @@ describe('ProjectBuilder', () => {
return builder.build({}).then(
() => fail('Unexpectedly resolved'),
error => {
expect(checkReqsSpy.check_android_target).toHaveBeenCalled();
expect(checkReqsSpy.check_android_target).toHaveBeenCalledWith(rootDir);
expect(error).toBe(testError);
}
);
Expand Down
4 changes: 3 additions & 1 deletion spec/unit/builders/builders.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ describe('builders', () => {

describe('getBuilder', () => {
it('should return an instance of ProjectBuilder when gradle is requested', () => {
const newBuilder = builders.getBuilder();
const root = 'FakeProjectRoot';
const newBuilder = builders.getBuilder(root);
expect(newBuilder).toEqual(jasmine.any(ProjectBuilder));
expect(newBuilder.root).toBe(root);
});

it('should throw an error if a builder cannot be instantiated', () => {
Expand Down
9 changes: 5 additions & 4 deletions spec/unit/check_reqs.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ describe('check_reqs', function () {
});

describe('get_target', function () {
const projectRoot = 'fakeProjectRoot';
var ConfigParser;
var getPreferenceSpy;
beforeEach(function () {
Expand All @@ -273,7 +274,7 @@ describe('check_reqs', function () {
});

it('should retrieve DEFAULT_TARGET_API', function () {
var target = check_reqs.get_target();
var target = check_reqs.get_target(projectRoot);
expect(target).toBeDefined();
expect(target).toContain('android-' + DEFAULT_TARGET_API);
});
Expand All @@ -282,7 +283,7 @@ describe('check_reqs', function () {
spyOn(fs, 'existsSync').and.returnValue(true);
getPreferenceSpy.and.returnValue(String(DEFAULT_TARGET_API + 1));

var target = check_reqs.get_target();
var target = check_reqs.get_target(projectRoot);

expect(getPreferenceSpy).toHaveBeenCalledWith('android-targetSdkVersion', 'android');
expect(target).toBe('android-' + (DEFAULT_TARGET_API + 1));
Expand All @@ -292,7 +293,7 @@ describe('check_reqs', function () {
spyOn(fs, 'existsSync').and.returnValue(true);
getPreferenceSpy.and.returnValue('android-99');

var target = check_reqs.get_target();
var target = check_reqs.get_target(projectRoot);

expect(getPreferenceSpy).toHaveBeenCalledWith('android-targetSdkVersion', 'android');
expect(target).toBe('android-' + DEFAULT_TARGET_API);
Expand All @@ -305,7 +306,7 @@ describe('check_reqs', function () {

getPreferenceSpy.and.returnValue(String(DEFAULT_TARGET_API - 1));

var target = check_reqs.get_target();
var target = check_reqs.get_target(projectRoot);

expect(getPreferenceSpy).toHaveBeenCalledWith('android-targetSdkVersion', 'android');
expect(target).toBe('android-' + DEFAULT_TARGET_API);
Expand Down