Skip to content

Commit

Permalink
fix(doctor): improve target and sdk compatibility detection on <8.2 (#…
Browse files Browse the repository at this point in the history
…5648)

* fix: limit android targets on cli <8.2

* chore: cleanup

* fix: validate all info not just targetSdk

* chore: bump doctor

Co-authored-by: Igor Randjelovic <rigor789@gmail.com>
  • Loading branch information
edusperoni and rigor789 committed Mar 11, 2022
1 parent 81cb9c3 commit fa257dd
Show file tree
Hide file tree
Showing 8 changed files with 141 additions and 32 deletions.
3 changes: 2 additions & 1 deletion lib/services/android-project-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,10 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject
}
);

this.$androidToolsInfo.validateTargetSdk({
this.$androidToolsInfo.validateInfo({
showWarningsAsErrors: true,
projectDir: projectData.projectDir,
validateTargetSdk: true,
});

return {
Expand Down
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"mobile"
],
"dependencies": {
"@nativescript/doctor": "2.0.7",
"@nativescript/doctor": "2.0.8",
"@nativescript/schematics-executor": "0.0.2",
"@rigor789/resolve-package-path": "^1.0.5",
"axios": "^0.21.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/doctor/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@nativescript/doctor",
"version": "2.0.7",
"version": "2.0.8",
"description": "Library that helps identifying if the environment can be used for development of {N} apps.",
"main": "src/index.js",
"types": "./typings/nativescript-doctor.d.ts",
Expand Down
25 changes: 18 additions & 7 deletions packages/doctor/src/android-tools-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,19 @@ export class AndroidToolsInfo implements NativeScriptDoctor.IAndroidToolsInfo {
"android-32",
];

if (runtimeVersion && semver.lt(semver.coerce(runtimeVersion), "6.1.0")) {
baseTargets.sort();
const indexOfSdk29 = baseTargets.indexOf("android-29");
baseTargets = baseTargets.slice(0, indexOfSdk29);
const isRuntimeVersionLessThan = (targetVersion: string) => {
return (
runtimeVersion &&
semver.lt(semver.coerce(runtimeVersion), targetVersion)
);
};

if (isRuntimeVersionLessThan("6.1.0")) {
// limit baseTargets to android-17 - android-28 if the runtime is < 6.1.0
baseTargets = baseTargets.slice(0, baseTargets.indexOf("android-29"));
} else if (isRuntimeVersionLessThan("8.2.0")) {
// limit baseTargets to android-17 - android-30 if the runtime is < 8.2.0
baseTargets = baseTargets.slice(0, baseTargets.indexOf("android-31"));
}

return baseTargets;
Expand Down Expand Up @@ -119,15 +128,16 @@ export class AndroidToolsInfo implements NativeScriptDoctor.IAndroidToolsInfo {
message = `You have to install version ${versionRangeMatches[1]}.`;
}

let invalidBuildToolsAdditionalMsg = `Run \`\$ ${this.getPathToSdkManagementTool()}\` from your command-line to install required \`Android Build Tools\`.`;
// let invalidBuildToolsAdditionalMsg = `Run \`\$ ${this.getPathToSdkManagementTool()}\` from your command-line to install required \`Android Build Tools\`.`;
let invalidBuildToolsAdditionalMsg = `Install the required build-tools through Android Studio.`;
if (!isAndroidHomeValid) {
invalidBuildToolsAdditionalMsg +=
" In case you already have them installed, make sure `ANDROID_HOME` environment variable is set correctly.";
" In case you already have them installed, make sure the `ANDROID_HOME` environment variable is set correctly.";
}

errors.push({
warning:
"You need to have the Android SDK Build-tools installed on your system. " +
"No compatible version of the Android SDK Build-tools are installed on your system. " +
message,
additionalInformation: invalidBuildToolsAdditionalMsg,
platforms: [Constants.ANDROID_PLATFORM_NAME],
Expand Down Expand Up @@ -504,6 +514,7 @@ export class AndroidToolsInfo implements NativeScriptDoctor.IAndroidToolsInfo {

private getMaxSupportedVersion(projectDir: string): number {
const supportedTargets = this.getSupportedTargets(projectDir);

return this.parseAndroidSdkString(
supportedTargets.sort()[supportedTargets.length - 1]
);
Expand Down
116 changes: 106 additions & 10 deletions packages/doctor/test/android-tools-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,25 @@ describe("androidToolsInfo", () => {
},
readDirectory: (path: string) => {
if (path.indexOf("build-tools") >= 0) {
return ["20.0.0", "27.0.3", "28.0.3", "29.0.1"];
return [
"20.0.0",
"27.0.3",
"28.0.3",
"29.0.1",
"30.0.0",
"31.0.0",
"32.0.0",
];
} else {
return ["android-16", "android-27", "android-28", "android-29"];
return [
"android-16",
"android-27",
"android-28",
"android-29",
"android-30",
"android-31",
"android-32",
];
}
},
};
Expand All @@ -58,34 +74,68 @@ describe("androidToolsInfo", () => {
return new AndroidToolsInfo(childProcess, fs, hostInfo, helpers);
};

describe("getToolsInfo", () => {
it("runtime 6.0.0", () => {
describe("getToolsInfo -> compileSdkVersion", () => {
it("runtime 6.0.0 - 28", () => {
const androidToolsInfo = getAndroidToolsInfo("6.0.0");
const toolsInfo = androidToolsInfo.getToolsInfo({ projectDir: "test" });

assert.equal(toolsInfo.compileSdkVersion, 28);
});

it("runtime 6.1.0", () => {
it("runtime 6.1.0 - 30", () => {
const androidToolsInfo = getAndroidToolsInfo("6.1.0");
const toolsInfo = androidToolsInfo.getToolsInfo({ projectDir: "test" });

assert.equal(toolsInfo.compileSdkVersion, 29);
assert.equal(toolsInfo.compileSdkVersion, 30);
});

it("runtime 8.1.1 - 30", () => {
const androidToolsInfo = getAndroidToolsInfo("8.1.1");
const toolsInfo = androidToolsInfo.getToolsInfo({ projectDir: "test" });

assert.equal(toolsInfo.compileSdkVersion, 30);
});

it("runtime 8.2.0 - 32", () => {
const androidToolsInfo = getAndroidToolsInfo("8.2.0");
const toolsInfo = androidToolsInfo.getToolsInfo({ projectDir: "test" });

assert.equal(toolsInfo.compileSdkVersion, 32);
});
});

describe("supportedAndroidSdks", () => {
it("should support android-17 - android-32", () => {
const min = 17;
const max = 32;
const assertSupportedRange = (
runtimeVersion: string,
min: number,
max: number
) => {
let cnt = 0;
const androidToolsInfo = getAndroidToolsInfo("6.5.0");
const androidToolsInfo = getAndroidToolsInfo(runtimeVersion);
const supportedTargets = androidToolsInfo.getSupportedTargets("test");
for (let i = 0; i < supportedTargets.length; i++) {
assert.equal(supportedTargets[i], `android-${min + i}`);
cnt = min + i;
}
assert.equal(cnt, max);
};

it("runtime 6.0.0 should support android-17 - android-28", () => {
const min = 17;
const max = 28;
assertSupportedRange("6.0.0", min, max);
});

it("runtime 8.1.0 should support android-17 - android-30", () => {
const min = 17;
const max = 30;
assertSupportedRange("8.1.0", min, max);
});

it("runtime 8.2.0 should support android-17 - android-32", () => {
const min = 17;
const max = 32;
assertSupportedRange("8.2.0", min, max);
});
});

Expand Down Expand Up @@ -295,6 +345,52 @@ describe("androidToolsInfo", () => {
);
});

describe("validataMaxSupportedTargetSdk", () => {
const testCases = [
{
runtimeVersion: "8.1.0",
targetSdk: 30,
expectWarning: false,
},
{
runtimeVersion: "8.1.0",
targetSdk: 31,
expectWarning: true,
},
{
runtimeVersion: "8.1.0",
targetSdk: 32,
expectWarning: true,
},
{
runtimeVersion: "8.2.0",
targetSdk: 32,
expectWarning: false,
},
];

testCases.forEach(({ runtimeVersion, targetSdk, expectWarning }) => {
it(`for runtime ${runtimeVersion} - and targetSdk ${targetSdk}`, () => {
const androidToolsInfo = getAndroidToolsInfo(runtimeVersion);
const actualWarnings = androidToolsInfo.validataMaxSupportedTargetSdk({
projectDir: "test",
targetSdk,
});
let expectedWarnings: NativeScriptDoctor.IWarning[] = [];

if (expectWarning) {
expectedWarnings.push({
additionalInformation: "",
platforms: ["Android"],
warning: `Support for the selected Android target SDK android-${targetSdk} is not verified. Your Android app might not work as expected.`,
});
}

assert.deepEqual(actualWarnings, expectedWarnings);
});
});
});

after(() => {
process.env["ANDROID_HOME"] = originalAndroidHome;
});
Expand Down
3 changes: 2 additions & 1 deletion packages/doctor/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
},
"include": [
"src/",
"test/"
"test/",
"typings/"
]
}
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -340,10 +340,10 @@
"resolved" "https://registry.npmjs.org/@kwsites/promise-deferred/-/promise-deferred-1.1.1.tgz"
"version" "1.1.1"

"@nativescript/doctor@2.0.7":
"integrity" "sha512-Pd3NlFGXN+SC03sBC/pj/9t5v5h7bh2xnbjbXHCo1MTjjm6fpLGFSxlCZRPPFgDIisf4OqGmEeIglNpy7CbyZg=="
"resolved" "https://registry.npmjs.org/@nativescript/doctor/-/doctor-2.0.7.tgz"
"version" "2.0.7"
"@nativescript/doctor@2.0.8":
"integrity" "sha512-/jVGBBBBY2BX1IwriDyXHNi0ZNAkSuzdDQuGY3nUl3BDLu5AM+FFg4qCG3D9IW664WLbA1KbJQd+HUSjRHM/ZQ=="
"resolved" "https://registry.npmjs.org/@nativescript/doctor/-/doctor-2.0.8.tgz"
"version" "2.0.8"
dependencies:
"lodash" "4.17.21"
"osenv" "0.1.5"
Expand Down

0 comments on commit fa257dd

Please sign in to comment.