Skip to content

Commit

Permalink
fix: move the temp zip file to .ask in order to integ with extension …
Browse files Browse the repository at this point in the history
…skill-pacakge validator feature (#375)

Co-authored-by: Shen Chen <[email protected]>
  • Loading branch information
ShenChen93 and Shen Chen authored Jan 25, 2021
1 parent c239d49 commit a3beab3
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 15 deletions.
3 changes: 2 additions & 1 deletion lib/controllers/skill-metadata-controller/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ module.exports = class SkillMetadataController {
return callback(createUploadErr);
}
// 2.zip skill package
zipUtils.createTempZip(skillPackageSrc, (zipErr, zipFilePath) => {
const outputDir = path.join(process.cwd(), '.ask');
zipUtils.createTempZip(skillPackageSrc, outputDir, (zipErr, zipFilePath) => {
if (zipErr) {
return callback(zipErr);
}
Expand Down
9 changes: 7 additions & 2 deletions lib/utils/zip-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,14 @@ module.exports = {
* @param {string} src The file path of resource want to zip
* @param {callback} callback { error, filePath }
*/
function createTempZip(src, callback) {
function createTempZip(src, outputDir, callback) {
if (!src) {
return callback('Zip file path must be set.');
}

if (!outputDir) {
return callback('Zip file output path must be set.');
}
fs.access(src, (fs.constants || fs).W_OK, (err) => {
if (err) {
return callback(`File access error. ${err}`);
Expand All @@ -30,10 +34,11 @@ function createTempZip(src, callback) {
Messenger.getInstance().debug(`The source file ${src} has already been compressed. Skip the zipping`);
return callback(null, src);
}
// Create the temp zip at the same level of the source file
const zipFilePath = tmp.tmpNameSync({
prefix: 'askcli_temp_',
postfix: '.zip',
dir: src
dir: outputDir
});
const writeStream = fs.createWriteStream(zipFilePath);
const archive = archiver('zip');
Expand Down
8 changes: 4 additions & 4 deletions test/unit/controller/skill-metadata-controller-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ describe('Controller test - skill metadata controller test', () => {
sinon.stub(SkillMetadataController.prototype, '_createUploadUrl').callsArgWith(0, null, {
uploadUrl: TEST_UPLOAD_URL
});
sinon.stub(zipUtils, 'createTempZip').callsArgWith(1, 'zipErr');
sinon.stub(zipUtils, 'createTempZip').callsArgWith(2, 'zipErr');
// call
skillMetaController.uploadSkillPackage(TEST_PATH, (err, res) => {
// verify
Expand All @@ -624,7 +624,7 @@ describe('Controller test - skill metadata controller test', () => {
sinon.stub(SkillMetadataController.prototype, '_createUploadUrl').callsArgWith(0, null, {
uploadUrl: TEST_UPLOAD_URL
});
sinon.stub(zipUtils, 'createTempZip').callsArgWith(1, null, TEST_PATH);
sinon.stub(zipUtils, 'createTempZip').callsArgWith(2, null, TEST_PATH);
sinon.stub(fs, 'removeSync');
sinon.stub(httpClient, 'putByUrl').callsArgWith(4, 'uploadErr');
// call
Expand All @@ -642,7 +642,7 @@ describe('Controller test - skill metadata controller test', () => {

it('| upload zip file meets error, expect callback error', (done) => {
// setup
sinon.stub(zipUtils, 'createTempZip').callsArgWith(1, null, TEST_PATH);
sinon.stub(zipUtils, 'createTempZip').callsArgWith(2, null, TEST_PATH);
sinon.stub(fs, 'readFileSync').withArgs(TEST_PATH).returns(TEST_FILE_CONTENT);
sinon.stub(SkillMetadataController.prototype, '_createUploadUrl').callsArgWith(0, null, {
uploadUrl: TEST_UPLOAD_URL
Expand All @@ -666,7 +666,7 @@ describe('Controller test - skill metadata controller test', () => {

it('| upload skill package succeeds, expect callback upload result', (done) => {
// setup
sinon.stub(zipUtils, 'createTempZip').callsArgWith(1, null, TEST_PATH);
sinon.stub(zipUtils, 'createTempZip').callsArgWith(2, null, TEST_PATH);
sinon.stub(fs, 'readFileSync').withArgs(TEST_PATH).returns(TEST_FILE_CONTENT);
sinon.stub(SkillMetadataController.prototype, '_createUploadUrl').callsArgWith(0, null, {
uploadUrl: TEST_UPLOAD_URL
Expand Down
26 changes: 18 additions & 8 deletions test/unit/utils/zip-utils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const CONSTANTS = require('@src/utils/constants');

describe('Utils test - zip utility', () => {
const TEST_SRC = 'TEST_SRC';
const TEST_OUTPUT_DIR = 'TEST_OUTPUT_DIR';
const TEST_ERROR = 'TEST_ERROR';
const TEST_ZIP_FILE_PATH = 'TEST_ZIP_FILE_PATH';
const TEST_REMOTE_ZIP_URL = 'TEST_REMOTE_ZIP_URL';
Expand All @@ -21,7 +22,7 @@ describe('Utils test - zip utility', () => {
sinon.stub(tmp, 'tmpNameSync').withArgs({
prefix: 'askcli_temp_',
postfix: '.zip',
dir: TEST_SRC
dir: TEST_OUTPUT_DIR
}).callsFake(() => TEST_ZIP_FILE_PATH);

new Messenger({});
Expand All @@ -34,19 +35,28 @@ describe('Utils test - zip utility', () => {

it('| call back error when src file is not set', (done) => {
// call
zipUtils.createTempZip(null, (error) => {
zipUtils.createTempZip(null, TEST_OUTPUT_DIR, (error) => {
// verify
expect(error).equal('Zip file path must be set.');
done();
});
});

it('| call back error when outputDir is not set', (done) => {
// call
zipUtils.createTempZip(TEST_SRC, null, (error) => {
// verify
expect(error).equal('Zip file output path must be set.');
done();
});
});

it('| call back error when access file fail', (done) => {
// setup
sinon.stub(fs, 'access').callsArgWith(2, TEST_ERROR);

// call
zipUtils.createTempZip(TEST_SRC, (error) => {
zipUtils.createTempZip(TEST_SRC, TEST_OUTPUT_DIR, (error) => {
// verify
expect(error).equal(`File access error. ${TEST_ERROR}`);
done();
Expand All @@ -61,7 +71,7 @@ describe('Utils test - zip utility', () => {
sinon.stub(Messenger.getInstance(), 'debug');

// call
zipUtils.createTempZip(src, (error, response) => {
zipUtils.createTempZip(src, TEST_OUTPUT_DIR, (error, response) => {
// verify
expect(Messenger.getInstance().debug.args[0][0]).equal(`The source file ${src} has already been compressed. Skip the zipping`);
expect(error).equal(null);
Expand All @@ -78,7 +88,7 @@ describe('Utils test - zip utility', () => {
sinon.stub(Messenger.getInstance(), 'debug');

// call
zipUtils.createTempZip(src, (error, response) => {
zipUtils.createTempZip(src, TEST_OUTPUT_DIR, (error, response) => {
// verify
expect(Messenger.getInstance().debug.args[0][0]).equal(`The source file ${src} has already been compressed. Skip the zipping`);
expect(error).equal(null);
Expand Down Expand Up @@ -122,7 +132,7 @@ describe('Utils test - zip utility', () => {
});
archiveOnStub.callsArgWith(1, 'error');
// call
proxyZipUtils.createTempZip(TEST_SRC, (error, response) => {
proxyZipUtils.createTempZip(TEST_SRC, TEST_OUTPUT_DIR, (error, response) => {
// verify
expect(fs.createWriteStream.args[0][0]).equal(TEST_ZIP_FILE_PATH);
expect(error).equal('Archive error. error');
Expand All @@ -139,7 +149,7 @@ describe('Utils test - zip utility', () => {
});
writeStreamOnStub.callsArgWith(1);
// call
proxyZipUtils.createTempZip(TEST_SRC, (error, response) => {
proxyZipUtils.createTempZip(TEST_SRC, TEST_OUTPUT_DIR, (error, response) => {
// verify
expect(fs.createWriteStream.args[0][0]).equal(TEST_ZIP_FILE_PATH);
expect(archiveFileStub.args[0][0]).equal(TEST_SRC);
Expand All @@ -158,7 +168,7 @@ describe('Utils test - zip utility', () => {
});
writeStreamOnStub.callsArgWith(1);
// call
proxyZipUtils.createTempZip(TEST_SRC, (error, response) => {
proxyZipUtils.createTempZip(TEST_SRC, TEST_OUTPUT_DIR, (error, response) => {
// verify
expect(fs.createWriteStream.args[0][0]).equal(TEST_ZIP_FILE_PATH);
expect(archiveGlobStub.args[0][0]).equal('**/*');
Expand Down

0 comments on commit a3beab3

Please sign in to comment.