Skip to content

Commit 38ba049

Browse files
Anatoly BolshakovEzzhevNikita
Anatoly Bolshakov
and
EzzhevNikita
authored
[CopyFilesV2] Considered delay between retries for cp action. Switched to node 6. (#15098) (#15102)
* Added delay for retries - cp action * Improve retry logic * Changed error messages and added fuction description * Revert "[CopyFilesV2] Migrated to Node10 (#14710)" This reverts commit e8abe8b. * Tests * Fixed tests Co-authored-by: Nikita Ezzhev <[email protected]> Co-authored-by: Nikita Ezzhev <[email protected]>
1 parent d4b9f8c commit 38ba049

18 files changed

+4143
-455
lines changed

‎Tasks/CopyFilesV2/Tests/L0.ts

+34-17
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ describe('CopyFiles L0 Suite', function () {
1010

1111
after(() => { });
1212

13-
it('copy files from srcdir to destdir', (done: Mocha.Done) => {
13+
it('copy files from srcdir to destdir', (done: MochaDone) => {
14+
this.timeout(1000);
1415

1516
let testPath = path.join(__dirname, 'L0copyAllFiles.js');
1617
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
@@ -49,7 +50,8 @@ describe('CopyFiles L0 Suite', function () {
4950
done();
5051
});
5152

52-
it('copy files from srcdir to destdir with brackets in src path', (done: Mocha.Done) => {
53+
it('copy files from srcdir to destdir with brackets in src path', (done: MochaDone) => {
54+
this.timeout(1000);
5355

5456
let testPath = path.join(__dirname, 'L0copyAllFilesWithBracketsInSrcPath.js');
5557
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
@@ -88,7 +90,8 @@ describe('CopyFiles L0 Suite', function () {
8890
done();
8991
});
9092

91-
it('copy files and subtract based on exclude pattern', (done: Mocha.Done) => {
93+
it('copy files and subtract based on exclude pattern', (done: MochaDone) => {
94+
this.timeout(1000);
9295

9396
let testPath = path.join(__dirname, 'L0copySubtractExclude.js');
9497
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
@@ -118,7 +121,8 @@ describe('CopyFiles L0 Suite', function () {
118121
done();
119122
});
120123

121-
it('fails if Contents not set', (done: Mocha.Done) => {
124+
it('fails if Contents not set', (done: MochaDone) => {
125+
this.timeout(1000);
122126

123127
let testPath = path.join(__dirname, 'L0failsIfContentsNotSet.js');
124128
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
@@ -129,7 +133,8 @@ describe('CopyFiles L0 Suite', function () {
129133
done();
130134
});
131135

132-
it('fails if SourceFolder not set', (done: Mocha.Done) => {
136+
it('fails if SourceFolder not set', (done: MochaDone) => {
137+
this.timeout(1000);
133138

134139
let testPath = path.join(__dirname, 'L0failsIfSourceFolderNotSet.js');
135140
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
@@ -140,7 +145,8 @@ describe('CopyFiles L0 Suite', function () {
140145
done();
141146
});
142147

143-
it('fails if TargetFolder not set', (done: Mocha.Done) => {
148+
it('fails if TargetFolder not set', (done: MochaDone) => {
149+
this.timeout(1000);
144150

145151
let testPath = path.join(__dirname, 'L0failsIfTargetFolderNotSet.js');
146152
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
@@ -151,7 +157,8 @@ describe('CopyFiles L0 Suite', function () {
151157
done();
152158
});
153159

154-
it('fails if SourceFolder not found', (done: Mocha.Done) => {
160+
it('fails if SourceFolder not found', (done: MochaDone) => {
161+
this.timeout(1000);
155162

156163
let testPath = path.join(__dirname, 'L0failsIfSourceFolderNotFound.js');
157164
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
@@ -162,7 +169,8 @@ describe('CopyFiles L0 Suite', function () {
162169
done();
163170
});
164171

165-
it('fails if target file is a directory', (done: Mocha.Done) => {
172+
it('fails if target file is a directory', (done: MochaDone) => {
173+
this.timeout(1000);
166174

167175
let testPath = path.join(__dirname, 'L0failsIfTargetFileIsDir.js');
168176
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
@@ -173,7 +181,8 @@ describe('CopyFiles L0 Suite', function () {
173181
done();
174182
});
175183

176-
it('skips if exists', (done: Mocha.Done) => {
184+
it('skips if exists', (done: MochaDone) => {
185+
this.timeout(1000);
177186

178187
let testPath = path.join(__dirname, 'L0skipsIfExists.js');
179188
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
@@ -197,7 +206,8 @@ describe('CopyFiles L0 Suite', function () {
197206
done();
198207
});
199208

200-
it('overwrites if specified', (done: Mocha.Done) => {
209+
it('overwrites if specified', (done: MochaDone) => {
210+
this.timeout(1000);
201211

202212
let testPath = path.join(__dirname, 'L0overwritesIfSpecified.js');
203213
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
@@ -221,7 +231,8 @@ describe('CopyFiles L0 Suite', function () {
221231
done();
222232
});
223233

224-
it('preserves timestamp if specified', (done: Mocha.Done) => {
234+
it('preserves timestamp if specified', (done: MochaDone) => {
235+
this.timeout(1000);
225236

226237
let testPath = path.join(__dirname, 'L0preservesTimestampIfSpecified.js');
227238
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
@@ -248,7 +259,8 @@ describe('CopyFiles L0 Suite', function () {
248259
done();
249260
});
250261

251-
it('cleans if specified', (done: Mocha.Done) => {
262+
it('cleans if specified', (done: MochaDone) => {
263+
this.timeout(1000);
252264

253265
let testPath = path.join(__dirname, 'L0cleansIfSpecified.js');
254266
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
@@ -278,7 +290,8 @@ describe('CopyFiles L0 Suite', function () {
278290
done();
279291
});
280292

281-
it('cleans if specified and target is file', (done: Mocha.Done) => {
293+
it('cleans if specified and target is file', (done: MochaDone) => {
294+
this.timeout(1000);
282295

283296
let testPath = path.join(__dirname, 'L0cleansIfSpecifiedAndTargetIsFile.js');
284297
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
@@ -305,7 +318,9 @@ describe('CopyFiles L0 Suite', function () {
305318
done();
306319
});
307320

308-
it('roots patterns', (done: Mocha.Done) => {
321+
it('roots patterns', (done: MochaDone) => {
322+
this.timeout(1000);
323+
309324
let testPath = path.join(__dirname, 'L0rootsPatterns.js');
310325
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
311326
runner.run();
@@ -325,7 +340,7 @@ describe('CopyFiles L0 Suite', function () {
325340
done();
326341
});
327342

328-
it('ignores errors during target folder creation if ignoreMakeDirErrors is true', (done: Mocha.Done) => {
343+
it('ignores errors during target folder creation if ignoreMakeDirErrors is true', (done: MochaDone) => {
329344
let testPath = path.join(__dirname, 'L0IgnoresMakeDirError.js');
330345
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
331346
runner.run();
@@ -343,7 +358,7 @@ describe('CopyFiles L0 Suite', function () {
343358
done();
344359
});
345360

346-
it('fails if there are errors during target folder creation if ignoreMakeDirErrors is false', (done: Mocha.Done) => {
361+
it('fails if there are errors during target folder creation if ignoreMakeDirErrors is false', (done: MochaDone) => {
347362
let testPath = path.join(__dirname, 'L0FailsIfThereIsMkdirError.js');
348363
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
349364
runner.run();
@@ -355,7 +370,9 @@ describe('CopyFiles L0 Suite', function () {
355370
});
356371

357372
if (process.platform == 'win32') {
358-
it('overwrites readonly', (done: Mocha.Done) => {
373+
it('overwrites readonly', (done: MochaDone) => {
374+
this.timeout(1000);
375+
359376
let testPath = path.join(__dirname, 'L0overwritesReadonly.js');
360377
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
361378
runner.run();

‎Tasks/CopyFilesV2/Tests/L0cleansIfSpecified.ts

+6-3
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,17 @@ runner.registerMockExport('stats', (itemPath: string) => {
4141
}
4242
});
4343
let origReaddirSync = fs.readdirSync;
44-
fs.readdirSync = (p) => {
44+
fs.readdirSync = (p: string | Buffer) => {
4545
console.log('HERE path ' + p);
46+
let result: string[];
4647
if (p == path.normalize('/destDir')) {
47-
return [ 'clean-subDir', 'clean-file.txt'] as any;
48+
result = [ 'clean-subDir', 'clean-file.txt' ];
4849
}
4950
else {
50-
return origReaddirSync(p);
51+
result = origReaddirSync(p);
5152
}
53+
54+
return result;
5255
}
5356

5457
// as a precaution, disable fs.chmodSync. it should not be called during this scenario.

‎Tasks/CopyFilesV2/Tests/L0preservesTimestampIfSpecified.ts

+2-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import fs = require('fs');
22
import mockanswer = require('azure-pipelines-task-lib/mock-answer');
33
import mockrun = require('azure-pipelines-task-lib/mock-run');
44
import path = require('path');
5-
const { promisify } = require('util')
65

76
let taskPath = path.join(__dirname, '..', 'copyfiles.js');
87
let runner: mockrun.TaskMockRunner = new mockrun.TaskMockRunner(taskPath);
@@ -47,9 +46,9 @@ runner.registerMockExport('stats', (itemPath: string) => {
4746
}
4847
});
4948

50-
fs.utimes = promisify(function (targetPath, atime, mtime, err) {
49+
fs.utimes = function (targetPath, atime, mtime, err) {
5150
console.log('Calling fs.utimes on', targetPath);
52-
});
51+
}
5352
runner.registerMock('fs', fs);
5453

5554
runner.run();

‎Tasks/CopyFilesV2/copyfiles.ts

+60-38
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import fs = require('fs');
22
import path = require('path');
33
import tl = require('azure-pipelines-task-lib/task');
4-
import { RetryOptions, RetryHelper } from './retryHelper';
4+
import { RetryOptions, RetryHelper } from './retrylogichelper';
55

66
/**
77
* Shows timestamp change operation results
@@ -37,6 +37,23 @@ function makeDirP(targetFolder: string, ignoreErrors: boolean): void {
3737
}
3838
}
3939

40+
/**
41+
* Gets stats for the provided path. Will ignore ENOENT error if ignoreEnoent is true.
42+
* If ignoreEnoent is false ENOENT will be thrown from the function.
43+
* @param path path for which methid will try to get tl.FsStats.
44+
* @param ignoreEnoent ignore ENOENT error during check of path stats.
45+
* @returns
46+
*/
47+
function stats(path: string, ignoreEnoent: boolean): tl.FsStats {
48+
try {
49+
return tl.stats(path);
50+
} catch (err) {
51+
if (err.code != 'ENOENT' && ignoreEnoent) {
52+
throw err;
53+
}
54+
}
55+
}
56+
4057
async function main(): Promise<void> {
4158
// we allow broken symlinks - since there could be broken symlinks found in source folder, but filtered by contents pattern
4259
const findOptions: tl.FindOptions = {
@@ -80,7 +97,7 @@ async function main(): Promise<void> {
8097
let allPaths: string[] = tl.find(sourceFolder, findOptions);
8198
let sourceFolderPattern = sourceFolder.replace('[', '[[]'); // directories can have [] in them, and they have special meanings as a pattern, so escape them
8299
let matchedPaths: string[] = tl.match(allPaths, contents, sourceFolderPattern); // default match options
83-
let matchedFiles: string[] = matchedPaths.filter((itemPath: string) => !tl.stats(itemPath).isDirectory()); // filter-out directories
100+
let matchedFiles: string[] = matchedPaths.filter((itemPath: string) => !stats(itemPath, false).isDirectory()); // filter-out directories
84101

85102
// copy the files to the target folder
86103
console.log(tl.loc('FoundNFiles', matchedFiles.length));
@@ -92,43 +109,40 @@ async function main(): Promise<void> {
92109

93110
// stat the targetFolder path
94111
let targetFolderStats: tl.FsStats;
95-
try {
96-
targetFolderStats = await retryHelper.RunWithRetrySingleArg<tl.FsStats, string>(
97-
() => tl.stats(targetFolder),
98-
targetFolder);
99-
} catch (err) {
100-
if (err.code != 'ENOENT') {
101-
throw err;
102-
}
103-
}
112+
targetFolderStats = await retryHelper.RunWithRetry<tl.FsStats>(
113+
() => stats(targetFolder, true),
114+
`stats for ${targetFolder}`
115+
);
104116

105117
if (targetFolderStats) {
106118
if (targetFolderStats.isDirectory()) {
107119
// delete the child items
108-
const folderItems: string[] = await retryHelper.RunWithRetrySingleArg<string[], string>(
120+
const folderItems: string[] = await retryHelper.RunWithRetry<string[]>(
109121
() => fs.readdirSync(targetFolder),
110-
targetFolder);
122+
`readdirSync for ${targetFolder}`
123+
);
111124

112125
for (let item of folderItems) {
113126
let itemPath = path.join(targetFolder, item);
114-
await retryHelper.RunWithRetrySingleArg<void, string>(() =>
127+
await retryHelper.RunWithRetry(() =>
115128
tl.rmRF(itemPath),
116-
targetFolder
129+
`delete of ${itemPath}`
117130
);
118131
}
119132
} else {
120-
await retryHelper.RunWithRetrySingleArg<void, string>(() =>
133+
await retryHelper.RunWithRetry(() =>
121134
tl.rmRF(targetFolder),
122-
targetFolder);
135+
`delete of ${targetFolder}`
136+
);
123137
}
124138
}
125139
}
126140

127141
// make sure the target folder exists
128-
await retryHelper.RunWithRetryMultiArgs<void, string, boolean>(() =>
142+
await retryHelper.RunWithRetry(() =>
129143
makeDirP(targetFolder, ignoreMakeDirErrors),
130-
targetFolder,
131-
ignoreMakeDirErrors);
144+
`makeDirP for ${targetFolder}`
145+
);
132146
try {
133147
let createdFolders: { [folder: string]: boolean } = {};
134148
for (let file of matchedFiles) {
@@ -150,23 +164,20 @@ async function main(): Promise<void> {
150164

151165
if (!createdFolders[targetDir]) {
152166
await retryHelper.RunWithRetry(
153-
() => makeDirP(targetDir, ignoreMakeDirErrors));
167+
() => makeDirP(targetDir, ignoreMakeDirErrors),
168+
`makeDirP for ${targetDir}`
169+
);
154170

155171
createdFolders[targetDir] = true;
156172
}
157173

158174
// stat the target
159175
let targetStats: tl.FsStats;
160176
if (!cleanTargetFolder) { // optimization - no need to check if relative target exists when CleanTargetFolder=true
161-
try {
162-
targetStats = await retryHelper.RunWithRetrySingleArg<tl.FsStats, string>(
163-
() => tl.stats(targetPath),
164-
targetPath);
165-
} catch (err) {
166-
if (err.code != 'ENOENT') {
167-
throw err;
168-
}
169-
}
177+
targetStats = await retryHelper.RunWithRetry<tl.FsStats>(
178+
() => stats(targetPath, true),
179+
`Stats for ${targetPath}`
180+
);
170181
}
171182

172183
// validate the target is not a directory
@@ -179,13 +190,17 @@ async function main(): Promise<void> {
179190
console.log(tl.loc('FileAlreadyExistAt', file, targetPath));
180191
} else { // copy
181192
console.log(tl.loc('CopyingTo', file, targetPath));
182-
tl.cp(file, targetPath, undefined, undefined, retryCount);
193+
await retryHelper.RunWithRetry(
194+
() => tl.cp(file, targetPath),
195+
`copy ${file} to ${targetPath}`
196+
);
183197
if (preserveTimestamp) {
184198
try {
185199
let fileStats;
186-
fileStats = await retryHelper.RunWithRetrySingleArg<tl.FsStats, string>(
187-
(file) => tl.stats(file),
188-
file);
200+
fileStats = await retryHelper.RunWithRetry<tl.FsStats>(
201+
() => stats(file, false),
202+
`stats for ${file}`
203+
);
189204
fs.utimes(targetPath, fileStats.atime, fileStats.mtime, (err) => {
190205
displayTimestampChangeResults(fileStats, err);
191206
});
@@ -214,15 +229,22 @@ async function main(): Promise<void> {
214229
// https://github.com/nodejs/node/blob/v5.x/deps/uv/src/win/fs.c#L1064
215230
tl.debug(`removing readonly attribute on '${targetPath}'`);
216231

217-
await retryHelper.RunWithRetrySingleArg<void, string>(
232+
await retryHelper.RunWithRetry(
218233
() => fs.chmodSync(targetPath, targetStats.mode | 146),
219-
targetPath);
234+
`chmodSync for ${targetPath}`
235+
);
220236
}
237+
await retryHelper.RunWithRetry(
238+
() => tl.cp(file, targetPath, "-f"),
239+
`copy ${file} to ${targetPath}`
240+
);
221241

222-
tl.cp(file, targetPath, "-f", undefined, retryCount);
223242
if (preserveTimestamp) {
224243
try {
225-
const fileStats = tl.stats(file);
244+
const fileStats = await retryHelper.RunWithRetry<tl.FsStats>(
245+
() => stats(file, false),
246+
`stats for ${file}`
247+
);
226248
fs.utimes(targetPath, fileStats.atime, fileStats.mtime, (err) => {
227249
displayTimestampChangeResults(fileStats, err);
228250
});

0 commit comments

Comments
 (0)