Skip to content

Commit

Permalink
Fix findCommonParent behaviour when NativeLibrariestPath targets a si…
Browse files Browse the repository at this point in the history
…ngle .so file (#12395)

* Fix findCommonParent behaviour when NativeLibrariestPath targets to single .so file
* Add Unit tests for utils
* Fix tests
* Rename unitTest to assertByExitCode
* Add timeout to spawnSync
* Add test for unit tests exit code

Co-authored-by: Alex Kharchenko <[email protected]>
  • Loading branch information
vaagnavanesyan and snowpardx authored Feb 26, 2020
1 parent b414a7a commit 5c985ad
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 2 deletions.
12 changes: 12 additions & 0 deletions Tasks/AppCenterDistributeV3/Tests/L0.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import * as path from 'path';
import * as assert from 'assert';
import * as ttm from 'vsts-task-lib/mock-test';
import { utilsUnitTests } from './L0UtilsUnitTests';
import { spawnSync } from 'child_process';

describe('AppCenterDistribute L0 Suite', function () {
before(() => {
Expand Down Expand Up @@ -310,4 +312,14 @@ describe('AppCenterDistribute L0 Suite', function () {
tr.run();
assert(tr.succeeded, 'task should have succeeded');
});

describe("Unit tests", function() {
it('Negative path: should keep exit code', function() {
const tp = path.join(__dirname, 'UnitTests', 'UnitTestsExitCodeIsKept.js');
const spawn = spawnSync('node', [tp], {timeout: 2000});
assert.equal(spawn.status, 1);
});

utilsUnitTests();
});
});
27 changes: 27 additions & 0 deletions Tasks/AppCenterDistributeV3/Tests/L0UtilsUnitTests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import * as path from 'path';
import * as assert from 'assert';
import {spawnSync} from 'child_process';

export function utilsUnitTests() {
describe('Utils unit tests', function () {
describe('findCommonParent', function() {
it('returns containing directory for single file', function() {
const tp = path.join(__dirname, 'UnitTests', 'Utils', 'FindCommonParentForSingleFileReturnsContainingDirectory.js');
const spawn = spawnSync('node', [tp], {timeout: 2000});
assert.equal(spawn.status, 0);
});

it('returns directory itself for single directory', function() {
const tp = path.join(__dirname, 'UnitTests', 'Utils', 'FindCommonParentForSingleDirectoryReturnsDirectoryItself.js');
const spawn = spawnSync('node', [tp], {timeout: 2000});
assert.equal(spawn.status, 0);
});

it('returns common parent for multiple values', function() {
const tp = path.join(__dirname, 'UnitTests', 'Utils', 'FindCommonParentForMultipleFilesReturnsCorrectResult.js');
const spawn = spawnSync('node', [tp], {timeout: 2000});
assert.equal(spawn.status, 0);
});
});
});
}
18 changes: 18 additions & 0 deletions Tasks/AppCenterDistributeV3/Tests/UnitTests/TestHelpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import * as assert from "assert";

/**
* Exit code is used to determine whether unit test passed or not.
* When executing code requires vsts-task-lib somewhere it makes exit code = 0 regardless whether exception was thrown.
* This helper allows to follow default NodeJS exit code behaviour when exception is thrown.
*/
export const assertByExitCode = {
equal: (actual, expected) => wrapAssertWithExitCode(assert.equal, actual, expected),
};

function wrapAssertWithExitCode(assert, ...args) {
try {
assert.apply(undefined, args);
} catch (error) {
process.exit(1);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { assertByExitCode } from "./TestHelpers";

// Verify that exit code doesn't overwritten
assertByExitCode.equal(true, false);
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import fs = require('fs');

import { findCommonParent } from '../../../utils';
import { assertByExitCode } from '../TestHelpers';

fs.lstatSync = (s: string) => {
const stat = {} as fs.Stats;
stat.isFile = () => s.endsWith('.so');
stat.isSymbolicLink = () => false;
return stat;
}
const expectedParentPath = "/a/b";
const actualParentPath = findCommonParent(["/a/b/c/x", "/a/b/c/y", "/a/b/d/z"]);
assertByExitCode.equal(actualParentPath, expectedParentPath);
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import fs = require('fs');

import { findCommonParent } from '../../../utils';
import { assertByExitCode } from '../TestHelpers';

fs.lstatSync = (s: string) => {
const stat = {} as fs.Stats;
stat.isFile = () => s.endsWith('.so');
stat.isSymbolicLink = () => false;
return stat;
}
const expectedParentPath = "/a/b/c";
const actualParentPath = findCommonParent(["/a/b/c"]);
assertByExitCode.equal(actualParentPath, expectedParentPath);
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import fs = require('fs');

import { findCommonParent } from '../../../utils';
import { assertByExitCode } from '../TestHelpers';

fs.lstatSync = (s: string) => {
const stat = {} as fs.Stats;
stat.isFile = () => s.endsWith('.so');
return stat;
}
const singleSoFilePath = "/a/b/c/symbol.so";
const expectedParentPath = "/a/b/c";
const actualParentPath = findCommonParent([singleSoFilePath]);
assertByExitCode.equal(actualParentPath, expectedParentPath);
2 changes: 1 addition & 1 deletion Tasks/AppCenterDistributeV3/task.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"author": "Microsoft Corporation",
"version": {
"Major": 3,
"Minor": 159,
"Minor": 160,
"Patch": 0
},
"releaseNotes": "Added support for forwarding Android mapping to App Center Diagnostics. Added missing descriptions.",
Expand Down
2 changes: 1 addition & 1 deletion Tasks/AppCenterDistributeV3/task.loc.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"author": "Microsoft Corporation",
"version": {
"Major": 3,
"Minor": 159,
"Minor": 160,
"Patch": 0
},
"releaseNotes": "ms-resource:loc.releaseNotes",
Expand Down
5 changes: 5 additions & 0 deletions Tasks/AppCenterDistributeV3/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ export function findCommonParent(list: string[]): string {
return null;
}

if(list.length === 1 && fs.lstatSync(list[0]).isFile()) {
// We expect that common parent for single file is its containing directory, not the file path itself
return path.dirname(list[0]);
}

let commonSegments: string[] = [];
let parentPath: string = null;

Expand Down

0 comments on commit 5c985ad

Please sign in to comment.