Skip to content

Fix update command and add tests. #3227

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 30, 2017
Merged

Fix update command and add tests. #3227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
82641ad
Fix update command and add tests.
KristianDD Nov 20, 2017
ce93e02
Fix comments.
KristianDD Nov 27, 2017
adca13b
Rewrite tests.
KristianDD Nov 28, 2017
6bf0f6d
Fix comments.
KristianDD Nov 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 75 additions & 46 deletions lib/commands/update.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as path from "path";
import * as shelljs from "shelljs";
import * as constants from "../constants";

export class UpdateCommand implements ICommand {
public allowedParameters: ICommandParameter[] = [];
Expand All @@ -15,49 +15,41 @@ export class UpdateCommand implements ICommand {
this.$projectData.initializeProjectData();
}

static readonly folders: string[] = [
constants.LIB_DIR_NAME,
constants.HOOKS_DIR_NAME,
constants.PLATFORMS_DIR_NAME,
constants.NODE_MODULES_FOLDER_NAME
];
static readonly tempFolder: string = ".tmp_backup";
static readonly updateFailMessage: string = "Could not update the project!";
static readonly backupFailMessage: string = "Could not backup project folders!";

public async execute(args: string[]): Promise<void> {
const folders = ["lib", "hooks", "platforms", "node_modules"];
const tmpDir = path.join(this.$projectData.projectDir, ".tmp_backup");
const tmpDir = path.join(this.$projectData.projectDir, UpdateCommand.tempFolder);

try {
shelljs.rm("-fr", tmpDir);
shelljs.mkdir(tmpDir);
shelljs.cp(path.join(this.$projectData.projectDir, "package.json"), tmpDir);
for (const folder of folders) {
const folderToCopy = path.join(this.$projectData.projectDir, folder);
if (this.$fs.exists(folderToCopy)) {
shelljs.cp("-rf", folderToCopy, tmpDir);
}
}
this.backup(tmpDir);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better API to understand is this.backup(source, destination), or if you use only source, why do you need a temp folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I send the parameter just to avoid repetition of:
const tmpDir = path.join(this.$projectData.projectDir, UpdateCommand.tempFolder);

} catch (error) {
this.$logger.error("Could not backup project folders!");
this.$logger.error(UpdateCommand.backupFailMessage);
this.$fs.deleteDirectory(tmpDir);
return;
}

try {
await this.executeCore(args, folders);
await this.executeCore(args);
} catch (error) {
shelljs.cp("-f", path.join(tmpDir, "package.json"), this.$projectData.projectDir);
for (const folder of folders) {
shelljs.rm("-rf", path.join(this.$projectData.projectDir, folder));

const folderToCopy = path.join(tmpDir, folder);

if (this.$fs.exists(folderToCopy)) {
shelljs.cp("-fr", folderToCopy, this.$projectData.projectDir);
}
}

this.$logger.error("Could not update the project!");
this.restoreBackup(tmpDir);
this.$logger.error(UpdateCommand.updateFailMessage);
} finally {
shelljs.rm("-fr", tmpDir);
this.$fs.deleteDirectory(tmpDir);
}
}

public async canExecute(args: string[]): Promise<boolean> {
for (const arg of args) {
const platform = arg.split("@")[0];
this.$platformService.validatePlatformInstalled(platform, this.$projectData);
const platforms = this.getPlatforms();

for (const platform of platforms.packagePlatforms) {
const platformData = this.$platformsData.getPlatformData(platform, this.$projectData);
const platformProjectService = platformData.platformProjectService;
await platformProjectService.validate(this.$projectData);
Expand All @@ -66,42 +58,79 @@ export class UpdateCommand implements ICommand {
return args.length < 2 && this.$projectData.projectDir !== "";
}

private async executeCore(args: string[], folders: string[]): Promise<void> {
let platforms = this.$platformService.getInstalledPlatforms(this.$projectData);
const availablePlatforms = this.$platformService.getAvailablePlatforms(this.$projectData);
const packagePlatforms: string[] = [];
private async executeCore(args: string[]): Promise<void> {
const platforms = this.getPlatforms();

for (const platform of availablePlatforms) {
for (const platform of _.xor(platforms.installed, platforms.packagePlatforms)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic has changed a bit here - instead of iterating over all availablePlatforms we are basically iterating over packagePlatforms because platforms.packagePlatforms is platforms.installed concatenated with packagePlatforms and the xor will basically only return packagePlatforms.
Is this expected

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this is intentional, because we are missing the following part:

if (platformVersion) {
 	packagePlatforms.push(platform);
 	this.$projectDataService.removeNSProperty(this.$projectData.projectDir, platformData.frameworkPackageName);
}

The if statement is moved to getPlatforms and we want to call removeNSProperty only for the ones that passed the condition.

The behavior should be the same.

const platformData = this.$platformsData.getPlatformData(platform, this.$projectData);
const platformVersion = this.$projectDataService.getNSValue(this.$projectData.projectDir, platformData.frameworkPackageName);
if (platformVersion) {
packagePlatforms.push(platform);
this.$projectDataService.removeNSProperty(this.$projectData.projectDir, platformData.frameworkPackageName);
}
this.$projectDataService.removeNSProperty(this.$projectData.projectDir, platformData.frameworkPackageName);
}

await this.$platformService.removePlatforms(platforms, this.$projectData);
await this.$platformService.removePlatforms(platforms.installed, this.$projectData);
await this.$pluginsService.remove("tns-core-modules", this.$projectData);
await this.$pluginsService.remove("tns-core-modules-widgets", this.$projectData);

for (const folder of folders) {
shelljs.rm("-fr", folder);
for (const folder of UpdateCommand.folders) {
this.$fs.deleteDirectory(path.join(this.$projectData.projectDir, folder));
}

platforms = platforms.concat(packagePlatforms);
if (args.length === 1) {
for (const platform of platforms) {
for (const platform of platforms.packagePlatforms) {
await this.$platformService.addPlatforms([platform + "@" + args[0]], this.$options.platformTemplate, this.$projectData, this.$options, this.$options.frameworkPath);
}

await this.$pluginsService.add("tns-core-modules@" + args[0], this.$projectData);
} else {
await this.$platformService.addPlatforms(platforms, this.$options.platformTemplate, this.$projectData, this.$options, this.$options.frameworkPath);
await this.$platformService.addPlatforms(platforms.packagePlatforms, this.$options.platformTemplate, this.$projectData, this.$options, this.$options.frameworkPath);
await this.$pluginsService.add("tns-core-modules", this.$projectData);
}

await this.$pluginsService.ensureAllDependenciesAreInstalled(this.$projectData);
}

private getPlatforms(): {installed: string[], packagePlatforms: string[]} {
const installedPlatforms = this.$platformService.getInstalledPlatforms(this.$projectData);
const availablePlatforms = this.$platformService.getAvailablePlatforms(this.$projectData);
const packagePlatforms: string[] = [];

for (const platform of availablePlatforms) {
const platformData = this.$platformsData.getPlatformData(platform, this.$projectData);
const platformVersion = this.$projectDataService.getNSValue(this.$projectData.projectDir, platformData.frameworkPackageName);
if (platformVersion) {
packagePlatforms.push(platform);
}
}

return {
installed: installedPlatforms,
packagePlatforms: installedPlatforms.concat(packagePlatforms)
};
}

private restoreBackup(tmpDir: string): void {
this.$fs.copyFile(path.join(tmpDir, constants.PACKAGE_JSON_FILE_NAME), this.$projectData.projectDir);
for (const folder of UpdateCommand.folders) {
this.$fs.deleteDirectory(path.join(this.$projectData.projectDir, folder));

const folderToCopy = path.join(tmpDir, folder);

if (this.$fs.exists(folderToCopy)) {
this.$fs.copyFile(folderToCopy, this.$projectData.projectDir);
}
}
}

private backup(tmpDir: string): void {
this.$fs.deleteDirectory(tmpDir);
this.$fs.createDirectory(tmpDir);
this.$fs.copyFile(path.join(this.$projectData.projectDir, constants.PACKAGE_JSON_FILE_NAME), tmpDir);
for (const folder of UpdateCommand.folders) {
const folderToCopy = path.join(this.$projectData.projectDir, folder);
if (this.$fs.exists(folderToCopy)) {
this.$fs.copyFile(folderToCopy, tmpDir);
}
}
}
}

$injector.registerCommand("update", UpdateCommand);
2 changes: 2 additions & 0 deletions lib/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ export const TEST_RUNNER_NAME = "nativescript-unit-test-runner";
export const LIVESYNC_EXCLUDED_FILE_PATTERNS = ["**/*.js.map", "**/*.ts"];
export const XML_FILE_EXTENSION = ".xml";
export const PLATFORMS_DIR_NAME = "platforms";
export const HOOKS_DIR_NAME = "hooks";
export const LIB_DIR_NAME = "lib";
export const CODE_SIGN_ENTITLEMENTS = "CODE_SIGN_ENTITLEMENTS";
export const AWAIT_NOTIFICATION_TIMEOUT_SECONDS = 9;
export const SRC_DIR = "src";
Expand Down
2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
"analyze": true,
"devDependencies": {
"@types/chai": "4.0.1",
"@types/sinon": "4.0.0",
"@types/chai-as-promised": "0.0.31",
"@types/chokidar": "1.6.0",
"@types/lockfile": "1.0.0",
Expand All @@ -104,6 +105,7 @@
"grunt-ts": "6.0.0-beta.16",
"istanbul": "0.4.5",
"mocha": "3.1.2",
"sinon": "4.1.2",
"should": "7.0.2",
"source-map-support": "^0.4.14",
"tslint": "5.4.3",
Expand Down
Loading