fix: Gradle Args parsing (#1606)

* fix: Gradle Args parsing

* refactor: Applied ARGVParser.parseArgsStringToArgv -> parseArgsStringToArgv suggestion

* test: Added deeper testing for gradle argument parsing
This commit is contained in:
Norman Breau 2023-04-22 17:00:51 -03:00 committed by GitHub
parent a62f699380
commit 3343c3bb34
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 158 additions and 15 deletions

View File

@ -21,6 +21,7 @@ const path = require('path');
const fs = require('fs');
const nopt = require('nopt');
const untildify = require('untildify');
const { parseArgsStringToArgv } = require('string-argv');
const Adb = require('./Adb');
@ -36,7 +37,11 @@ function parseOpts (options, resolvedTarget, projectRoot) {
minSdkVersion: String,
maxSdkVersion: String,
targetSdkVersion: String,
// This needs to be an array since nopts will parse its entries as further options for this process
// It will be an array of 1 string: [ "string args" ]
gradleArg: [String, Array],
keystore: path,
alias: String,
storePassword: String,
@ -58,7 +63,8 @@ function parseOpts (options, resolvedTarget, projectRoot) {
if (options.argv.maxSdkVersion) { ret.extraArgs.push('-PcdvMaxSdkVersion=' + options.argv.maxSdkVersion); }
if (options.argv.targetSdkVersion) { ret.extraArgs.push('-PcdvTargetSdkVersion=' + options.argv.targetSdkVersion); }
if (options.argv.gradleArg) {
ret.extraArgs = ret.extraArgs.concat(options.argv.gradleArg);
const gradleArgs = parseArgsStringToArgv(options.argv.gradleArg[0]);
ret.extraArgs = ret.extraArgs.concat(gradleArgs);
}
const packageArgs = {};

View File

@ -85,7 +85,13 @@ class ProjectBuilder {
}
getArgs (cmd, opts) {
let args;
let args = [
'-b', path.join(this.root, 'build.gradle')
];
if (opts.extraArgs) {
args = args.concat(opts.extraArgs);
}
let buildCmd = cmd;
if (opts.packageType === PackageType.BUNDLE) {
if (cmd === 'release') {
@ -93,8 +99,6 @@ class ProjectBuilder {
} else if (cmd === 'debug') {
buildCmd = ':app:bundleDebug';
}
args = [buildCmd, '-b', path.join(this.root, 'build.gradle')];
} else {
if (cmd === 'release') {
buildCmd = 'cdvBuildRelease';
@ -102,14 +106,12 @@ class ProjectBuilder {
buildCmd = 'cdvBuildDebug';
}
args = [buildCmd, '-b', path.join(this.root, 'build.gradle')];
if (opts.arch) {
args.push('-PcdvBuildArch=' + opts.arch);
}
}
args.push.apply(args, opts.extraArgs);
args.push(buildCmd);
return args;
}
@ -318,6 +320,8 @@ class ProjectBuilder {
const wrapper = path.join(this.root, 'gradlew');
const args = this.getArgs(opts.buildType === 'debug' ? 'debug' : 'release', opts);
events.emit('verbose', `Running Gradle Build: ${wrapper} ${args.join(' ')}`);
try {
return await execa(wrapper, args, { stdio: 'inherit', cwd: path.resolve(this.root) });
} catch (error) {

14
package-lock.json generated
View File

@ -18,6 +18,7 @@
"nopt": "^7.1.0",
"properties-parser": "^0.3.1",
"semver": "^7.3.8",
"string-argv": "^0.3.1",
"untildify": "^4.0.0",
"which": "^3.0.0"
},
@ -4358,6 +4359,14 @@
"integrity": "sha512-D9cPgkvLlV3t3IzL0D0YLvGA9Ahk4PcvVwUbN0dSGr1aP0Nrt4AEnTUbuGvquEC0mA64Gqt1fzirlRs5ibXx8g==",
"dev": true
},
"node_modules/string-argv": {
"version": "0.3.1",
"resolved": "https://registry.npmjs.org/string-argv/-/string-argv-0.3.1.tgz",
"integrity": "sha512-a1uQGz7IyVy9YwhqjZIZu1c8JO8dNIe20xBmSS6qu9kv++k3JGzCVmprbNN5Kn+BgzD5E7YYwg1CcjuJMRNsvg==",
"engines": {
"node": ">=0.6.19"
}
},
"node_modules/string-width": {
"version": "4.2.3",
"resolved": "https://registry.npmjs.org/string-width/-/string-width-4.2.3.tgz",
@ -8118,6 +8127,11 @@
"integrity": "sha512-D9cPgkvLlV3t3IzL0D0YLvGA9Ahk4PcvVwUbN0dSGr1aP0Nrt4AEnTUbuGvquEC0mA64Gqt1fzirlRs5ibXx8g==",
"dev": true
},
"string-argv": {
"version": "0.3.1",
"resolved": "https://registry.npmjs.org/string-argv/-/string-argv-0.3.1.tgz",
"integrity": "sha512-a1uQGz7IyVy9YwhqjZIZu1c8JO8dNIe20xBmSS6qu9kv++k3JGzCVmprbNN5Kn+BgzD5E7YYwg1CcjuJMRNsvg=="
},
"string-width": {
"version": "4.2.3",
"resolved": "https://registry.npmjs.org/string-width/-/string-width-4.2.3.tgz",

View File

@ -34,6 +34,7 @@
"nopt": "^7.1.0",
"properties-parser": "^0.3.1",
"semver": "^7.3.8",
"string-argv": "^0.3.1",
"untildify": "^4.0.0",
"which": "^3.0.0"
},

116
spec/unit/build.spec.js Normal file
View File

@ -0,0 +1,116 @@
/*
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 rewire = require('rewire');
const builders = require('../../lib/builders/builders');
describe('build', () => {
let build;
const builder = builders.getBuilder('FakeRootPath');
beforeEach(() => {
build = rewire('../../lib/build');
build.__set__({
events: jasmine.createSpyObj('eventsSpy', ['emit'])
});
// run needs `this` to behave like an Api instance
build.run = build.run.bind({
_builder: builder
});
spyOn(builder, 'build').and.returnValue(Promise.resolve({
paths: ['fake.apk'],
buildtype: 'debug'
}));
});
describe('argument parsing', () => {
let prepEnvSpy;
beforeEach(() => {
prepEnvSpy = spyOn(builder, 'prepEnv').and.returnValue(Promise.resolve());
});
describe('gradleArg', () => {
const baseOptions = {
packageType: 'apk',
arch: undefined,
prepEnv: undefined,
buildType: 'debug'
};
it('can parse single gradle argument', async () => {
await build.run({
argv: [
'node',
'--gradleArg=--stacktrace'
]
});
expect(prepEnvSpy).toHaveBeenCalledWith({
...baseOptions,
extraArgs: ['--stacktrace']
});
});
it('can parse multiple gradle arguments', async () => {
await build.run({
argv: [
'node',
'--gradleArg=--stacktrace --info'
]
});
expect(prepEnvSpy).toHaveBeenCalledWith({
...baseOptions,
extraArgs: ['--stacktrace', '--info']
});
});
it('can parse multiple gradle arguments with strings', async () => {
await build.run({
argv: [
'node',
'--gradleArg=--testArg="hello world"'
]
});
expect(prepEnvSpy).toHaveBeenCalledWith({
...baseOptions,
extraArgs: ['--testArg="hello world"']
});
});
it('gradle args will split when necessary', async () => {
await build.run({
argv: [
'node',
'--gradleArg=--warning-mode all'
]
});
expect(prepEnvSpy).toHaveBeenCalledWith({
...baseOptions,
extraArgs: ['--warning-mode', 'all']
});
});
});
});
});

View File

@ -53,40 +53,40 @@ describe('ProjectBuilder', () => {
it('should set release argument', () => {
const args = builder.getArgs('release', {});
expect(args[0]).toBe('cdvBuildRelease');
expect(args[args.length - 1]).toBe('cdvBuildRelease');
});
it('should set debug argument', () => {
const args = builder.getArgs('debug', {});
expect(args[0]).toBe('cdvBuildDebug');
expect(args[args.length - 1]).toBe('cdvBuildDebug');
});
it('should set apk release', () => {
const args = builder.getArgs('release', {
packageType: 'apk'
});
expect(args[0]).withContext(args).toBe('cdvBuildRelease');
expect(args[args.length - 1]).withContext(args).toBe('cdvBuildRelease');
});
it('should set apk debug', () => {
const args = builder.getArgs('debug', {
packageType: 'apk'
});
expect(args[0]).withContext(args).toBe('cdvBuildDebug');
expect(args[args.length - 1]).withContext(args).toBe('cdvBuildDebug');
});
it('should set bundle release', () => {
const args = builder.getArgs('release', {
packageType: 'bundle'
});
expect(args[0]).withContext(args).toBe(':app:bundleRelease');
expect(args[args.length - 1]).withContext(args).toBe(':app:bundleRelease');
});
it('should set bundle debug', () => {
const args = builder.getArgs('debug', {
packageType: 'bundle'
});
expect(args[0]).withContext(args).toBe(':app:bundleDebug');
expect(args[args.length - 1]).withContext(args).toBe(':app:bundleDebug');
});
it('should add architecture if it is passed', () => {
@ -100,14 +100,14 @@ describe('ProjectBuilder', () => {
const args = builder.getArgs('clean', {
packageType: 'apk'
});
expect(args[0]).toBe('clean');
expect(args[args.length - 1]).toBe('clean');
});
it('should clean bundle', () => {
const args = builder.getArgs('clean', {
packageType: 'bundle'
});
expect(args[0]).toBe('clean');
expect(args[args.length - 1]).toBe('clean');
});
describe('should accept extra arguments', () => {
@ -176,6 +176,7 @@ describe('ProjectBuilder', () => {
it('should reject if the spawn fails', () => {
const errorMessage = 'Test error';
execaSpy.and.rejectWith(new Error(errorMessage));
builder.getArgs.and.returnValue([]);
return builder.build({}).then(
() => fail('Unexpectedly resolved'),
@ -192,6 +193,7 @@ describe('ProjectBuilder', () => {
ProjectBuilder.__set__('check_reqs', checkReqsSpy);
checkReqsSpy.check_android_target.and.resolveTo();
execaSpy.and.rejectWith(testError);
builder.getArgs.and.returnValue([]);
return builder.build({}).then(
() => fail('Unexpectedly resolved'),