From 86d07c9a9c6f4fc4bf551fcdaa224e57d0c55265 Mon Sep 17 00:00:00 2001 From: Zhenya Ragulin Date: Fri, 30 Oct 2020 16:11:07 -0600 Subject: [PATCH 1/4] Don't wait for aborted builds forever --- src/build.js | 12 +++++++++--- test/build.test.js | 26 ++++++++++++++++++++++++++ yarn.lock | 36 +++++++++++++++++++++++++++++++++++- 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/build.js b/src/build.js index e01c42d..be21de0 100644 --- a/src/build.js +++ b/src/build.js @@ -29,7 +29,7 @@ const describeBuild = async ({ appSlug, buildSlug, client }) => { const followBuild = async ({ appSlug, buildSlug, client }, options = {}) => { let lastActive = Date.now(); let timestamp; - + let attributes; do { if (timestamp) { await sleep(options.interval || 5000); @@ -61,11 +61,17 @@ const followBuild = async ({ appSlug, buildSlug, client }, options = {}) => { lastActive = now; } + // Sometimes build might have empty log_chunks, have is_archived set to false, have non-empty timestamp, but be aborted + // (because of bitrise timeout for example). Don't follow such builds forever: + attributes = await describeBuild({appSlug, buildSlug, client}); + if (attributes.status === 3 && !response.data.is_archived && response.data.log_chunks.length === 0) { + process.stdout.write('Build has been aborted, not polling logs any more\n'); + throw new Error(`Build ${appSlug}/${buildSlug} aborted`); + } + timestamp = response.data.timestamp; } while (timestamp); - const attributes = await describeBuild({ appSlug, buildSlug, client }); - if (attributes.status > 1) { throw new Error(`Build ${appSlug}/${buildSlug} failed`); } diff --git a/test/build.test.js b/test/build.test.js index c3e2c24..5381586 100644 --- a/test/build.test.js +++ b/test/build.test.js @@ -105,6 +105,32 @@ test.serial('following a failed build that has not finished prints the log outpu } }); +test.serial('following an aborted build with is_archived=false prints the log output and then errors', async (test) => { + const { appSlug, build, buildSlug, client } = test.context; + const logChunks = [ + 'line one', + 'line two', + 'line three' + ]; + const buildStub = stubGetBuild({ appSlug, axios: client, buildSlug }); + buildStub.build.status = 3; + stubBuildLogStream({ appSlug, axios: client, buildSlug, logChunks }); + // Cause timers to execute immediately + const clock = sinon.stub(global, 'setTimeout').callsArg(0); + const write = sinon.stub(process.stdout, 'write'); + + try { + await test.throwsAsync(() => build.follow(), null, 'Build has been aborted, not polling logs any more\n'); + + for (const chunk of logChunks) { + sinon.assert.calledWithExactly(write, chunk); + } + } finally { + clock.restore(); + write.restore(); + } +}); + test.serial('following a successful build that has already finished prints the log output', async (test) => { const { appSlug, build, buildSlug, client } = test.context; const logText = 'some log text'; diff --git a/yarn.lock b/yarn.lock index bbb7985..d39abab 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2094,11 +2094,18 @@ follow-redirects@1.5.10: dependencies: debug "=3.1.0" -for-in@^1.0.2: +for-in@^1.0.1, for-in@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/for-in/-/for-in-1.0.2.tgz#81068d295a8142ec0ac726c6e2200c30fb6d5e80" integrity sha1-gQaNKVqBQuwKxybG4iAMMPttXoA= +for-own@^0.1.4: + version "0.1.5" + resolved "https://registry.yarnpkg.com/for-own/-/for-own-0.1.5.tgz#5265c681a4f294dabbf17c9509b6763aa84510ce" + integrity sha1-UmXGgaTylNq78XyVCbZ2OqhFEM4= + dependencies: + for-in "^1.0.1" + foreground-child@^1.5.3, foreground-child@^1.5.6: version "1.5.6" resolved "https://registry.yarnpkg.com/foreground-child/-/foreground-child-1.5.6.tgz#4fd71ad2dfde96789b980a5c0a295937cb2f5ce9" @@ -3131,6 +3138,14 @@ load-json-file@^5.2.0: strip-bom "^3.0.0" type-fest "^0.3.0" +locate-path@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/locate-path/-/locate-path-2.0.0.tgz#2b568b265eec944c6d9c0de9c3dbbbca0354cd8e" + integrity sha1-K1aLJl7slExtnA3pw9u7ygNUzY4= + dependencies: + p-locate "^2.0.0" + path-exists "^3.0.0" + locate-path@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/locate-path/-/locate-path-3.0.0.tgz#dbec3b3ab759758071b58fe59fc41871af21400e" @@ -3768,6 +3783,13 @@ p-finally@^1.0.0: resolved "https://registry.yarnpkg.com/p-finally/-/p-finally-1.0.0.tgz#3fbcfb15b899a44123b34b6dcc18b724336a2cae" integrity sha1-P7z7FbiZpEEjs0ttzBi3JDNqLK4= +p-limit@^1.1.0: + version "1.3.0" + resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-1.3.0.tgz#b86bd5f0c25690911c7590fcbfc2010d54b3ccb8" + integrity sha512-vvcXsLAJ9Dr5rQOPk7toZQZJApBl2K4J6dANSsEuh6QI41JYcsS/qhTGa9ErIUUgK3WNQoJYvylxvjqmiqEA9Q== + dependencies: + p-try "^1.0.0" + p-limit@^2.0.0, p-limit@^2.2.0: version "2.2.1" resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-2.2.1.tgz#aa07a788cc3151c939b5131f63570f0dd2009537" @@ -3775,6 +3797,13 @@ p-limit@^2.0.0, p-limit@^2.2.0: dependencies: p-try "^2.0.0" +p-locate@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/p-locate/-/p-locate-2.0.0.tgz#20a0103b222a70c8fd39cc2e580680f3dde5ec43" + integrity sha1-IKAQOyIqcMj9OcwuWAaA893l7EM= + dependencies: + p-limit "^1.1.0" + p-locate@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/p-locate/-/p-locate-3.0.0.tgz#322d69a05c0264b25997d9f40cd8a891ab0064a4" @@ -3794,6 +3823,11 @@ p-map@^2.0.0: resolved "https://registry.yarnpkg.com/p-map/-/p-map-2.1.0.tgz#310928feef9c9ecc65b68b17693018a665cea175" integrity sha512-y3b8Kpd8OAN444hxfBbFfj1FY/RjtTd8tzYwhUqNYXx0fXx2iX4maP4Qr6qhIKbQXI02wTLAda4fYUbDagTUFw== +p-try@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/p-try/-/p-try-1.0.0.tgz#cbc79cdbaf8fd4228e13f621f2b1a237c1b207b3" + integrity sha1-y8ec26+P1CKOE/Yh8rGiN8GyB7M= + p-try@^2.0.0: version "2.2.0" resolved "https://registry.yarnpkg.com/p-try/-/p-try-2.2.0.tgz#cb2868540e313d61de58fafbe35ce9004d5540e6" From 62c1bc6ae8040c13d9689753e1b62a2ce2654805 Mon Sep 17 00:00:00 2001 From: Zhenya Ragulin Date: Mon, 2 Nov 2020 16:20:06 -0700 Subject: [PATCH 2/4] Address comment, fix unit test --- src/build.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/build.js b/src/build.js index be21de0..85d1b93 100644 --- a/src/build.js +++ b/src/build.js @@ -63,7 +63,7 @@ const followBuild = async ({ appSlug, buildSlug, client }, options = {}) => { // Sometimes build might have empty log_chunks, have is_archived set to false, have non-empty timestamp, but be aborted // (because of bitrise timeout for example). Don't follow such builds forever: - attributes = await describeBuild({appSlug, buildSlug, client}); + attributes = await describeBuild({ appSlug, buildSlug, client }); if (attributes.status === 3 && !response.data.is_archived && response.data.log_chunks.length === 0) { process.stdout.write('Build has been aborted, not polling logs any more\n'); throw new Error(`Build ${appSlug}/${buildSlug} aborted`); @@ -71,7 +71,9 @@ const followBuild = async ({ appSlug, buildSlug, client }, options = {}) => { timestamp = response.data.timestamp; } while (timestamp); - + if (!attributes) { + attributes = await describeBuild({appSlug, buildSlug, client}); + } if (attributes.status > 1) { throw new Error(`Build ${appSlug}/${buildSlug} failed`); } From 6d1632f12140f91e4fa307f6b6619fbbfcd341c1 Mon Sep 17 00:00:00 2001 From: Zhenya Ragulin Date: Tue, 3 Nov 2020 08:47:50 -0700 Subject: [PATCH 3/4] Fix linting and coverage --- src/build.js | 3 ++- test/build.test.js | 15 +++------------ test/stubs.js | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/build.js b/src/build.js index 85d1b93..3dfd6b5 100644 --- a/src/build.js +++ b/src/build.js @@ -65,6 +65,7 @@ const followBuild = async ({ appSlug, buildSlug, client }, options = {}) => { // (because of bitrise timeout for example). Don't follow such builds forever: attributes = await describeBuild({ appSlug, buildSlug, client }); if (attributes.status === 3 && !response.data.is_archived && response.data.log_chunks.length === 0) { + console.log('inside'); process.stdout.write('Build has been aborted, not polling logs any more\n'); throw new Error(`Build ${appSlug}/${buildSlug} aborted`); } @@ -72,7 +73,7 @@ const followBuild = async ({ appSlug, buildSlug, client }, options = {}) => { timestamp = response.data.timestamp; } while (timestamp); if (!attributes) { - attributes = await describeBuild({appSlug, buildSlug, client}); + attributes = await describeBuild({ appSlug, buildSlug, client }); } if (attributes.status > 1) { throw new Error(`Build ${appSlug}/${buildSlug} failed`); diff --git a/test/build.test.js b/test/build.test.js index 5381586..8f2df35 100644 --- a/test/build.test.js +++ b/test/build.test.js @@ -5,7 +5,7 @@ const test = require('ava'); const uuid = require('uuid/v4'); const { DateTime } = require('luxon'); -const { stubAbortBuild, stubArchivedBuildLog, stubBuildLogStream, stubGetBuild, stubListArtifacts } = require('./stubs'); +const { stubAbortBuild, stubArchivedBuildLog, stubBuildLogStream, stubGetBuild, stubListArtifacts, stubEmptyNonArchivedBuildLog } = require('./stubs'); test.beforeEach((test) => { const appSlug = uuid(); @@ -107,24 +107,15 @@ test.serial('following a failed build that has not finished prints the log outpu test.serial('following an aborted build with is_archived=false prints the log output and then errors', async (test) => { const { appSlug, build, buildSlug, client } = test.context; - const logChunks = [ - 'line one', - 'line two', - 'line three' - ]; const buildStub = stubGetBuild({ appSlug, axios: client, buildSlug }); buildStub.build.status = 3; - stubBuildLogStream({ appSlug, axios: client, buildSlug, logChunks }); + stubEmptyNonArchivedBuildLog({ appSlug, axios: client, buildSlug }); // Cause timers to execute immediately const clock = sinon.stub(global, 'setTimeout').callsArg(0); const write = sinon.stub(process.stdout, 'write'); try { - await test.throwsAsync(() => build.follow(), null, 'Build has been aborted, not polling logs any more\n'); - - for (const chunk of logChunks) { - sinon.assert.calledWithExactly(write, chunk); - } + await test.throwsAsync(() => build.follow(), { message: `Build ${buildStub.build.slug}/${buildStub.build.build_slug} aborted` }); } finally { clock.restore(); write.restore(); diff --git a/test/stubs.js b/test/stubs.js index 24237f8..c0ac93d 100644 --- a/test/stubs.js +++ b/test/stubs.js @@ -64,6 +64,20 @@ exports.stubArchivedBuildLog = ({ appSlug, axios, buildSlug, logText }) => { }); }; +exports.stubEmptyNonArchivedBuildLog = ({ appSlug, axios, buildSlug }) => { + const logUrl = 'http://localhost/logs/example.txt'; + const stub = getStub(axios, 'get'); + + stub.withArgs(`/apps/${appSlug}/builds/${buildSlug}/log`).resolves({ + data: { + expiring_raw_log_url: logUrl, + is_archived: false, + log_chunks: [] + }, + status: 200 + }); +}; + exports.stubBuildLogStream = ({ appSlug, axios, buildSlug, logChunks }) => { const logStub = getStub(axios, 'get'); From 13e8f5ea214c7b460de50cc95f47872c22e1d5de Mon Sep 17 00:00:00 2001 From: Zhenya Ragulin Date: Tue, 3 Nov 2020 09:32:32 -0700 Subject: [PATCH 4/4] Forgot to remove console.log --- src/build.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/build.js b/src/build.js index 3dfd6b5..878691f 100644 --- a/src/build.js +++ b/src/build.js @@ -65,7 +65,6 @@ const followBuild = async ({ appSlug, buildSlug, client }, options = {}) => { // (because of bitrise timeout for example). Don't follow such builds forever: attributes = await describeBuild({ appSlug, buildSlug, client }); if (attributes.status === 3 && !response.data.is_archived && response.data.log_chunks.length === 0) { - console.log('inside'); process.stdout.write('Build has been aborted, not polling logs any more\n'); throw new Error(`Build ${appSlug}/${buildSlug} aborted`); }