Skip to content

Fix ArchiveFilesV2 task to properly handle adding single files to existing archives #21036

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.

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
27 changes: 27 additions & 0 deletions Tasks/ArchiveFilesV2/Tests/L0.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,5 +190,32 @@ describe('ArchiveFiles L0 Suite', function () {
assert(tr.stdout.indexOf('Creating archive') > -1, 'Should have tried to create archive');
assert(fs.existsSync(expectedArchivePath), `Should have successfully created the archive at ${expectedArchivePath}, instead directory contents are ${fs.readdirSync(path.dirname(expectedArchivePath))}`);
});

it('Successfully adds a single file to an existing archive', async function () {
this.timeout(5000);
process.env['archiveType'] = 'zip';
process.env['archiveFile'] = 'singleFileArchive.zip';
process.env['includeRootFolder'] = 'false';
process.env['replaceExistingArchive'] = 'false';
expectedArchivePath = path.join(__dirname, 'test_output', 'singleFileArchive.zip');

// Create the initial archive
let tp1: string = path.join(__dirname, 'L0CreateArchive.js');
let tr1: ttm.MockTestRunner = new ttm.MockTestRunner(tp1);
await tr1.runAsync();
assert(fs.existsSync(expectedArchivePath), 'Should have created the initial archive');

// Add a single file to the existing archive
let tp2: string = path.join(__dirname, 'L0AddSingleFileToExisting.js');
let tr2: ttm.MockTestRunner = new ttm.MockTestRunner(tp2);
await tr2.runAsync();

assert(tr2.succeeded, 'Task should have succeeded');
assert(tr2.stdout.indexOf('Adding to existing archive') > -1 ||
tr2.stdout.indexOf('Adding file to archive') > -1 ||
tr2.stdout.indexOf('Archiving file:') > -1,
'Should have added file to archive');
assert(fs.existsSync(expectedArchivePath), 'Archive should still exist');
});
}
});
17 changes: 17 additions & 0 deletions Tasks/ArchiveFilesV2/Tests/L0AddSingleFileToExisting.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import tmrm = require('azure-pipelines-task-lib/mock-run');
import path = require('path');

let taskPath = path.join(__dirname, '..', 'archivefiles.js');
let tmr: tmrm.TaskMockRunner = new tmrm.TaskMockRunner(taskPath);

process.env['AGENT_TEMPDIRECTORY'] = path.join(__dirname, 'test_temp');

// First, set up to create an initial archive
tmr.setInput('rootFolderOrFile', path.join(__dirname, 'test_folder', 'a', 'abc.txt'));
tmr.setInput('includeRootFolder', process.env['includeRootFolder']);
tmr.setInput('archiveType', process.env['archiveType']);
tmr.setInput('archiveFile', path.join(__dirname, 'test_output', process.env['archiveFile']));
tmr.setInput('replaceExistingArchive', process.env['replaceExistingArchive'] || 'false');
tmr.setInput('tarCompression', 'gz');

tmr.run(true);
15 changes: 10 additions & 5 deletions Tasks/ArchiveFilesV2/archivefiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,17 @@ function findFiles(): string[] {
}
return [path.basename(rootFolderOrFile)];
} else {
var fullPaths: string[] = tl.ls('-A', [rootFolderOrFile]);
var baseNames: string[] = [];
for (var i = 0; i < fullPaths.length; i++) {
baseNames[i] = path.basename(fullPaths[i]);
// Check if rootFolderOrFile is a file
if (fs.existsSync(rootFolderOrFile) && fs.statSync(rootFolderOrFile).isFile()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot, could you please add an additional check there with the feature flag DistributedTasks.Task.ArchiveOneFileWithRootFolderInput to ensure it is executed only when this flag is enabled by using tl.getPipelineFeature?

Copy link
Author

Choose a reason for hiding this comment

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

I've added the feature flag check as requested. The single file handling in the findFiles() function now only works when the DistributedTasks.Task.ArchiveOneFileWithRootFolderInput feature flag is enabled. When the flag is disabled, it falls back to the original behavior. Changes are in commit 481a343.

return [path.basename(rootFolderOrFile)];
} else {
var fullPaths: string[] = tl.ls('-A', [rootFolderOrFile]);
var baseNames: string[] = [];
for (var i = 0; i < fullPaths.length; i++) {
baseNames[i] = path.basename(fullPaths[i]);
}
return baseNames;
}
return baseNames;
}
}

Expand Down