Skip to content

Commit 1a881a1

Browse files
Dimitar TachevDimitarTachev
Dimitar Tachev
authored and
DimitarTachev
committed
chore: fix pull request comments
1 parent dcc78bf commit 1a881a1

File tree

5 files changed

+38
-37
lines changed

5 files changed

+38
-37
lines changed

‎lib/common

‎lib/constants.ts

+5
Original file line numberDiff line numberDiff line change
@@ -207,4 +207,9 @@ export class Hooks {
207207
public static createProject = "createProject";
208208
}
209209

210+
export class AndroidBuildDefaults {
211+
public static GradleVersion = "4.4";
212+
public static GradleAndroidPluginVersion = "3.1.2";
213+
}
214+
210215
export const PACKAGE_PLACEHOLDER_NAME = "__PACKAGE__";

‎lib/node-package-manager.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ export class NodePackageManager implements INodePackageManager {
123123
const url = `https://registry.npmjs.org/${packageName}`;
124124
this.$logger.trace(`Trying to get data from npm registry for package ${packageName}, url is: ${url}`);
125125
const responseData = (await this.$httpClient.httpRequest(url)).body;
126-
this.$logger.trace(`Successfully received data from npm registry for package ${packageName}.`);
126+
this.$logger.trace(`Successfully received data from npm registry for package ${packageName}. Response data is: ${responseData}`);
127127
const jsonData = JSON.parse(responseData);
128128
this.$logger.trace(`Successfully parsed data from npm registry for package ${packageName}.`);
129129
return jsonData;

‎lib/services/android-plugin-build-service.ts

+24-26
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as path from "path";
2-
import { MANIFEST_FILE_NAME, INCLUDE_GRADLE_NAME, ASSETS_DIR, RESOURCES_DIR, TNS_ANDROID_RUNTIME_NAME } from "../constants";
2+
import { MANIFEST_FILE_NAME, INCLUDE_GRADLE_NAME, ASSETS_DIR, RESOURCES_DIR, TNS_ANDROID_RUNTIME_NAME, AndroidBuildDefaults } from "../constants";
33
import { getShortPluginName, hook } from "../common/helpers";
44
import { Builder, parseString } from "xml2js";
55
import { ILogger } from "log4js";
@@ -24,7 +24,8 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
2424
private $logger: ILogger,
2525
private $npm: INodePackageManager,
2626
private $projectDataService: IProjectDataService,
27-
private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants) { }
27+
private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants,
28+
private $errors: IErrors) { }
2829

2930
private static MANIFEST_ROOT = {
3031
$: {
@@ -170,16 +171,16 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
170171
public async buildAar(options: IBuildOptions): Promise<boolean> {
171172
this.validateOptions(options);
172173
const manifestFilePath = this.getManifest(options.platformsAndroidDirPath);
173-
const androidSourceSetDirectories = this.getAndroidSourceDirectories(options.platformsAndroidDirPath);
174-
const shouldBuildAar = !!manifestFilePath || androidSourceSetDirectories.length > 0;
174+
const androidSourceDirectories = this.getAndroidSourceDirectories(options.platformsAndroidDirPath);
175+
const shouldBuildAar = !!manifestFilePath || androidSourceDirectories.length > 0;
175176

176177
if (shouldBuildAar) {
177178
const shortPluginName = getShortPluginName(options.pluginName);
178179
const pluginTempDir = path.join(options.tempPluginDirPath, shortPluginName);
179180
const pluginTempMainSrcDir = path.join(pluginTempDir, "src", "main");
180181

181182
await this.updateManifest(manifestFilePath, pluginTempMainSrcDir, shortPluginName);
182-
this.copySourceSetDirectories(androidSourceSetDirectories, pluginTempMainSrcDir);
183+
this.copySourceSetDirectories(androidSourceDirectories, pluginTempMainSrcDir);
183184
await this.setupGradle(pluginTempDir, options.platformsAndroidDirPath, options.projectDir);
184185
await this.buildPlugin({ pluginDir: pluginTempDir, pluginName: options.pluginName });
185186
this.copyAar(shortPluginName, pluginTempDir, options.aarOutputDir);
@@ -197,9 +198,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
197198
try {
198199
androidManifestContent = this.$fs.readText(manifestFilePath);
199200
} catch (err) {
200-
throw new Error(
201-
`Failed to fs.readFileSync the manifest file located at ${manifestFilePath}`
202-
);
201+
this.$errors.failWithoutHelp(`Failed to fs.readFileSync the manifest file located at ${manifestFilePath}. Error is: ${err.toString()}`);
203202
}
204203

205204
updatedManifestContent = await this.updateManifestContent(androidManifestContent, defaultPackageName);
@@ -211,15 +210,13 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
211210
try {
212211
this.$fs.writeFile(pathToTempAndroidManifest, updatedManifestContent);
213212
} catch (e) {
214-
throw new Error(`Failed to write the updated AndroidManifest in the new location - ${pathToTempAndroidManifest}`);
213+
this.$errors.failWithoutHelp(`Failed to write the updated AndroidManifest in the new location - ${pathToTempAndroidManifest}. Error is: ${e.toString()}`);
215214
}
216215
}
217216

218217
private copySourceSetDirectories(androidSourceSetDirectories: string[], pluginTempMainSrcDir: string): void {
219218
for (const dir of androidSourceSetDirectories) {
220-
const dirNameParts = dir.split(path.sep);
221-
// get only the last subdirectory of the entire path string. e.g. 'res', 'java', etc.
222-
const dirName = dirNameParts[dirNameParts.length - 1];
219+
const dirName = path.basename(dir);
223220
const destination = path.join(pluginTempMainSrcDir, dirName);
224221

225222
this.$fs.ensureDirectoryExists(destination);
@@ -272,7 +269,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
272269
}
273270

274271
private replaceGradleVersion(pluginTempDir: string, version: string): void {
275-
const gradleVersion = version || "4.4";
272+
const gradleVersion = version || AndroidBuildDefaults.GradleVersion;
276273
const gradleVersionPlaceholder = "{{runtimeGradleVersion}}";
277274
const gradleWrapperPropertiesPath = path.join(pluginTempDir, "gradle", "wrapper", "gradle-wrapper.properties");
278275

@@ -281,7 +278,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
281278

282279
private replaceGradleAndroidPluginVersion(buildGradlePath: string, version: string): void {
283280
const gradleAndroidPluginVersionPlaceholder = "{{runtimeAndroidPluginVersion}}";
284-
const gradleAndroidPluginVersion = version || "3.1.2";
281+
const gradleAndroidPluginVersion = version || AndroidBuildDefaults.GradleAndroidPluginVersion;
285282

286283
this.replaceFileContent(buildGradlePath, gradleAndroidPluginVersionPlaceholder, gradleAndroidPluginVersion);
287284
}
@@ -297,10 +294,10 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
297294
const includeGradlePath = path.join(platformsAndroidDirPath, INCLUDE_GRADLE_NAME);
298295
if (this.$fs.exists(includeGradlePath)) {
299296
const includeGradleContent = this.$fs.readText(includeGradlePath);
300-
const repositoriesAndDependenciesScopes = this.getIncludeGradleCompileDependenciesScope(includeGradleContent);
297+
const compileDependencies = this.getIncludeGradleCompileDependenciesScope(includeGradleContent);
301298

302-
if (repositoriesAndDependenciesScopes.length > 0) {
303-
this.$fs.appendFile(buildGradlePath, "\n" + repositoriesAndDependenciesScopes.join("\n"));
299+
if (compileDependencies.length) {
300+
this.$fs.appendFile(buildGradlePath, "\n" + compileDependencies.join("\n"));
304301
}
305302
}
306303
}
@@ -315,10 +312,10 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
315312
this.$fs.copyFile(pathToBuiltAar, path.join(aarOutputDir, `${shortPluginName}.aar`));
316313
}
317314
} catch (e) {
318-
throw new Error(`Failed to copy built aar to destination. ${e.message}`);
315+
this.$errors.failWithoutHelp(`Failed to copy built aar to destination. ${e.message}`);
319316
}
320317
} else {
321-
throw new Error(`No built aar found at ${pathToBuiltAar}`);
318+
this.$errors.failWithoutHelp(`No built aar found at ${pathToBuiltAar}`);
322319
}
323320
}
324321

@@ -336,7 +333,7 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
336333
try {
337334
includeGradleFileContent = this.$fs.readFile(includeGradleFilePath).toString();
338335
} catch (err) {
339-
throw new Error(`Failed to fs.readFileSync the include.gradle file located at ${includeGradleFilePath}`);
336+
this.$errors.failWithoutHelp(`Failed to fs.readFileSync the include.gradle file located at ${includeGradleFilePath}. Error is: ${err.toString()}`);
340337
}
341338

342339
const productFlavorsScope = this.getScope("productFlavors", includeGradleFileContent);
@@ -347,7 +344,8 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
347344

348345
return true;
349346
} catch (e) {
350-
throw new Error(`Failed to write the updated include.gradle in - ${includeGradleFilePath}`);
347+
this.$errors.failWithoutHelp(`Failed to write the updated include.gradle ` +
348+
`in - ${includeGradleFilePath}. Error is: ${e.toString()}`);
351349
}
352350
}
353351

@@ -375,13 +373,13 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
375373
try {
376374
await this.$childProcess.spawnFromEvent(gradlew, localArgs, "close", { cwd: pluginBuildSettings.pluginDir });
377375
} catch (err) {
378-
throw new Error(`Failed to build plugin ${pluginBuildSettings.pluginName} : \n${err}`);
376+
this.$errors.failWithoutHelp(`Failed to build plugin ${pluginBuildSettings.pluginName} : \n${err}`);
379377
}
380378
}
381379

382380
private validateOptions(options: IBuildOptions): void {
383381
if (!options) {
384-
throw new Error("Android plugin cannot be built without passing an 'options' object.");
382+
this.$errors.failWithoutHelp("Android plugin cannot be built without passing an 'options' object.");
385383
}
386384

387385
if (!options.pluginName) {
@@ -393,19 +391,19 @@ export class AndroidPluginBuildService implements IAndroidPluginBuildService {
393391
}
394392

395393
if (!options.tempPluginDirPath) {
396-
throw new Error("Android plugin cannot be built without passing the path to a directory where the temporary project should be built.");
394+
this.$errors.failWithoutHelp("Android plugin cannot be built without passing the path to a directory where the temporary project should be built.");
397395
}
398396

399397
this.validatePlatformsAndroidDirPathOption(options);
400398
}
401399

402400
private validatePlatformsAndroidDirPathOption(options: IBuildOptions): void {
403401
if (!options) {
404-
throw new Error("Android plugin cannot be built without passing an 'options' object.");
402+
this.$errors.failWithoutHelp("Android plugin cannot be built without passing an 'options' object.");
405403
}
406404

407405
if (!options.platformsAndroidDirPath) {
408-
throw new Error("Android plugin cannot be built without passing the path to the platforms/android dir.");
406+
this.$errors.failWithoutHelp("Android plugin cannot be built without passing the path to the platforms/android dir.");
409407
}
410408
}
411409

‎test/services/android-plugin-build-service.ts

+7-9
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { AndroidPluginBuildService } from "../../lib/services/android-plugin-build-service";
22
import { assert } from "chai";
3-
import { INCLUDE_GRADLE_NAME } from "../../lib/constants";
3+
import { INCLUDE_GRADLE_NAME, AndroidBuildDefaults } from "../../lib/constants";
44
import { getShortPluginName } from "../../lib/common/helpers";
55
import * as FsLib from "../../lib/common/file-system";
66
import * as path from "path";
@@ -34,7 +34,7 @@ describe('androidPluginBuildService', () => {
3434
spawnFromEventCalled = false;
3535
tempFolder = temp.mkdirSync("androidPluginBuildService-temp");
3636
pluginFolder = temp.mkdirSync("androidPluginBuildService-plugin");
37-
setupDI(options);
37+
createTestInjector(options);
3838
setupPluginFolders(options);
3939

4040
return {
@@ -46,7 +46,7 @@ describe('androidPluginBuildService', () => {
4646
};
4747
}
4848

49-
function setupDI(options: {
49+
function createTestInjector(options: {
5050
addProjectRuntime?: boolean
5151
latestRuntimeGradleVersion?: string,
5252
latestRuntimeGradleAndroidVersion?: string,
@@ -242,7 +242,7 @@ describe('androidPluginBuildService', () => {
242242
assert.isTrue(spawnFromEventCalled);
243243
});
244244

245-
it('builds aar with the specified runtime gradle versions when project runtime has gradle versions', async () => {
245+
it('builds aar with the specified runtime gradle versions when the project runtime has gradle versions', async () => {
246246
const expectedGradleVersion = "2.2.2";
247247
const expectedAndroidVersion = "3.3";
248248
const config: IBuildOptions = setup({
@@ -264,9 +264,7 @@ describe('androidPluginBuildService', () => {
264264
assert.isTrue(spawnFromEventCalled);
265265
});
266266

267-
it('builds aar with a hardcoded runtime gradle versions when a project runtime and the latest runtime do not have versions specified', async () => {
268-
const expectedGradleVersion = "4.4";
269-
const expectedAndroidVersion = "3.1.2";
267+
it('builds aar with the hardcoded gradle versions when the project runtime and the latest runtime do not have versions specified', async () => {
270268
const config: IBuildOptions = setup({
271269
addManifest: true,
272270
addProjectDir: true,
@@ -281,8 +279,8 @@ describe('androidPluginBuildService', () => {
281279
const actualAndroidVersion = getGradleAndroidPluginVersion();
282280
const actualGradleVersion = getGradleVersion();
283281

284-
assert.equal(actualGradleVersion, expectedGradleVersion);
285-
assert.equal(actualAndroidVersion, expectedAndroidVersion);
282+
assert.equal(actualGradleVersion, AndroidBuildDefaults.GradleVersion);
283+
assert.equal(actualAndroidVersion, AndroidBuildDefaults.GradleAndroidPluginVersion);
286284
assert.isTrue(spawnFromEventCalled);
287285
});
288286
});

0 commit comments

Comments
 (0)