Skip to content

Commit

Permalink
fix: Archive the whole folder where the .trace file is located (#1244)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mykola-mokhnach
mykola-mokhnach authored Oct 13, 2020
1 parent 5b1f2d9 commit d4fa723
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 44 deletions.
94 changes: 51 additions & 43 deletions lib/commands/performance.js
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import _ from 'lodash';
import path from 'path';
import { fs, zip, logger, util } from 'appium-support';
import { fs, zip, logger, util, tempDir } from 'appium-support';
import { SubProcess, exec } from 'teen_process';
import log from '../logger';
import { encodeBase64OrUpload } from '../utils';
import { waitForCondition } from 'asyncbox';
import os from 'os';
import B from 'bluebird';

const commands = {};

Expand All @@ -17,7 +17,7 @@ const DEFAULT_TIMEOUT_MS = 5 * 60 * 1000;
const STOP_TIMEOUT_MS = 3 * 60 * 1000;
const STARTUP_TIMEOUT_MS = 60 * 1000;
const DEFAULT_PROFILE_NAME = 'Activity Monitor';
const DEFAULT_EXT = 'trace';
const DEFAULT_EXT = '.trace';
const DEFAULT_PID = 'current';
const INSTRUMENTS = 'instruments';
const XCRUN = 'xcrun';
Expand Down Expand Up @@ -52,12 +52,13 @@ async function requireInstruments () {


class PerfRecorder {
constructor (reportPath, udid, opts = {}) {
constructor (reportRoot, udid, opts = {}) {
this._process = null;
this._reportPath = reportPath;
this._zippedReportPath = '';
this._timeout = (opts.timeout && opts.timeout > 0) ? opts.timeout : DEFAULT_TIMEOUT_MS;
this._profileName = opts.profileName || DEFAULT_PROFILE_NAME;
this._reportPath = path.resolve(reportRoot,
`appium_perf__${this._profileName.replace(/\W/g, '_')}__${Date.now()}${DEFAULT_EXT}`);
this._pid = opts.pid;
this._udid = udid;
this._logger = logger.getLogger(
Expand All @@ -74,24 +75,27 @@ class PerfRecorder {
}

async getZippedReportPath () {
if (await fs.exists(this._zippedReportPath)) {
return this._zippedReportPath;
}
const originalReportPath = await this.getOriginalReportPath();
if (!originalReportPath) {
return '';
}
const zippedReportPath = originalReportPath.replace(`.${DEFAULT_EXT}`, '.zip');
// This is to prevent possible race conditions, because the archive operation
// could be pretty time-intensive
if (!this._archivePromise) {
this._archivePromise = zip.toArchive(zippedReportPath, {
cwd: originalReportPath,
});
this._archivePromise = (async () => {
const originalReportPath = await this.getOriginalReportPath();
if (!originalReportPath) {
return '';
}
const zippedReportPath = await tempDir.path({
prefix: path.parse(originalReportPath).name,
suffix: '.zip'
});
await zip.toArchive(zippedReportPath, {
cwd: path.dirname(this._reportPath),
});
await fs.rimraf(path.dirname(this._reportPath));
this._zippedReportPath = zippedReportPath;
return this._zippedReportPath;
})();
}
await this._archivePromise;
this._zippedReportPath = zippedReportPath;
return this._zippedReportPath;
return await this._archivePromise;
}

isRunning () {
Expand All @@ -106,18 +110,23 @@ class PerfRecorder {
} catch (ign) {}
}
this._process = null;
const performCleanup = async () => {
try {
await B.all([this._zippedReportPath, path.dirname(this._reportPath)]
.filter(Boolean).map((x) => fs.rimraf(x)));
} catch (e) {
this._logger.warn(e.message);
}
};
if (this._archivePromise) {
this._archivePromise
// eslint-disable-next-line promise/prefer-await-to-then
.then(() => fs.rimraf(this._zippedReportPath))
.finally(() => {
.finally(async () => {
await performCleanup();
this._archivePromise = null;
})
.catch(() => {});
} else {
await fs.rimraf(this._zippedReportPath);
}
await fs.rimraf(this._reportPath);
await performCleanup();
return '';
}

Expand Down Expand Up @@ -201,7 +210,7 @@ class PerfRecorder {
const listProfilesCommand = toolName === XCTRACE
? `${XCRUN} ${XCTRACE} list templates`
: `${INSTRUMENTS} -s`;
this._logger.errorAndThrow(`There is no .${DEFAULT_EXT} file found for performance profile ` +
this._logger.errorAndThrow(`There is no ${DEFAULT_EXT} file found for performance profile ` +
`'${this._profileName}'. Make sure the profile is supported on this device. ` +
`You could use '${listProfilesCommand}' command to see the list of all available profiles. ` +
`Check the server log for more details`);
Expand Down Expand Up @@ -268,23 +277,17 @@ commands.mobileStartPerfRecord = async function mobileStartPerfRecord (opts = {}
} = opts;

if (!_.isEmpty(this._perfRecorders)) {
const recorders = this._perfRecorders
.filter((x) => x.profileName === profileName);
if (!_.isEmpty(recorders)) {
for (const recorder of recorders) {
if (recorder.isRunning()) {
log.debug(`Performance recorder for '${profileName}' on device '${this.opts.device.udid}' ` +
` is already running. Doing nothing`);
return;
}
_.pull(this._perfRecorders, recorder);
await recorder.stop(true);
for (const recorder of this._perfRecorders.filter((x) => x.profileName === profileName)) {
if (recorder.isRunning()) {
log.debug(`Performance recorder for '${profileName}' on device '${this.opts.device.udid}' ` +
` is already running. Doing nothing`);
return;
}
_.pull(this._perfRecorders, recorder);
await recorder.stop(true);
}
}

const reportPath = path.resolve(os.tmpdir(),
`appium_perf_${profileName.replace(/\W/g, '_')}_${util.uuidV4().substring(0, 8)}.${DEFAULT_EXT}`);
let realPid;
if (pid) {
if (_.toLower(pid) === DEFAULT_PID) {
Expand All @@ -294,7 +297,7 @@ commands.mobileStartPerfRecord = async function mobileStartPerfRecord (opts = {}
realPid = pid;
}
}
const recorder = new PerfRecorder(reportPath, this.opts.device.udid, {
const recorder = new PerfRecorder(await tempDir.openDir(), this.opts.device.udid, {
timeout: parseInt(timeout, 10),
profileName,
pid: parseInt(realPid, 10),
Expand Down Expand Up @@ -355,13 +358,18 @@ commands.mobileStopPerfRecord = async function mobileStopPerfRecord (opts = {})
log.errorAndThrow(`There are no records for performance profile '${profileName}' ` +
`and device ${this.opts.device.udid}. Have you started the profiling before?`);
}
const resultPath = await recorders[0].stop();

const recorder = _.first(recorders);
const resultPath = await recorder.stop();
if (!await fs.exists(resultPath)) {
log.errorAndThrow(`There is no .${DEFAULT_EXT} file found for performance profile '${profileName}' ` +
log.errorAndThrow(`There is no ${DEFAULT_EXT} file found for performance profile '${profileName}' ` +
`and device ${this.opts.device.udid}. Make sure the selected profile is supported on this device`);
}

return await encodeBase64OrUpload(resultPath, remotePath, opts);
const result = await encodeBase64OrUpload(resultPath, remotePath, opts);
_.pull(this._perfRecorders, recorder);
await fs.rimraf(resultPath);
return result;
};


Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"appium-ios-simulator": "^3.25.1",
"appium-remote-debugger": "^8.13.0",
"appium-support": "^2.47.1",
"appium-webdriveragent": "^2.23.1",
"appium-webdriveragent": "^2.26.1",
"appium-xcode": "^3.8.0",
"async-lock": "^1.0.0",
"asyncbox": "^2.3.1",
Expand Down

0 comments on commit d4fa723

Please sign in to comment.