From 423556c9da0403fe84ecc928af1d2b303287740a Mon Sep 17 00:00:00 2001 From: Rodion Abdurakhimov Date: Tue, 5 Jan 2021 00:15:38 +0200 Subject: [PATCH 1/5] feat: migrate logs to 'debug' package --- lib/config.js | 3 +- lib/logger.js | 26 ++++++++++++++++ lib/result.js | 9 +++--- lib/test.js | 16 +++++++--- package.json | 1 + .../result-command-empty-branch-checks.js | 1 + test/fixtures/http/result-command-positive.js | 1 + .../http/result-command-wrong-package-json.js | 1 + test/fixtures/http/test-command-positive.js | 1 + test/logger.js | 31 +++++++++++++++++++ 10 files changed, 81 insertions(+), 9 deletions(-) create mode 100644 lib/logger.js create mode 100644 test/logger.js diff --git a/lib/config.js b/lib/config.js index c83bc79..4137c68 100644 --- a/lib/config.js +++ b/lib/config.js @@ -2,6 +2,7 @@ const joi = require('joi') const path = require('path') +const debug = require('./logger')('wiby:config') const dependentSchema = joi.object({ repository: joi.string().uri({ @@ -32,7 +33,7 @@ exports.validate = ({ config: configFilePath }) => { throw result.error } - console.log(`✅ ${configFilePath}`) + debug(`✅ ${configFilePath}`) return result.value } diff --git a/lib/logger.js b/lib/logger.js new file mode 100644 index 0000000..248e0ae --- /dev/null +++ b/lib/logger.js @@ -0,0 +1,26 @@ +const debugPkg = require('debug') + +/** + * Factory function for 'debug' package. + * + * Accepts namespace for setting up 'debug'. Writes to stdout by default. + * In case stderr required - pass writeToStdOut = false. + * + * Retuns debug log function + */ +module.exports = function debug (namespace = 'wiby:general', writeToStdOut = true) { + const logger = debugPkg(namespace) + + if (writeToStdOut) { + logger.log = console.log.bind(console) + } + + return logger +} + +/** + * Enables 'debug' logs for specified namespace + */ +module.exports.enableLogs = function enableLogs (namespace = 'wiby:*') { + debugPkg.enable(namespace) +} diff --git a/lib/result.js b/lib/result.js index 4d8af14..347539f 100644 --- a/lib/result.js +++ b/lib/result.js @@ -1,16 +1,17 @@ const test = require('./test') const github = require('./github') const gitURLParse = require('git-url-parse') +const debug = require('./logger')('wiby:result') module.exports = async function ({ dependents }) { const parentPkgJSON = await test.getLocalPackageJSON() const parentPkgInfo = gitURLParse(parentPkgJSON.repository.url) - console.log(`Parent module: ${parentPkgInfo.owner}/${parentPkgJSON.name}`) + debug(`Parent module: ${parentPkgInfo.owner}/${parentPkgJSON.name}`) for (const { repository: url } of dependents) { const dependentPkgInfo = gitURLParse(url) const dependentPkgJSON = await github.getPackageJson(dependentPkgInfo.owner, dependentPkgInfo.name) - console.log(`Dependent module: ${dependentPkgInfo.owner}/${dependentPkgInfo.name}`) + debug(`Dependent module: ${dependentPkgInfo.owner}/${dependentPkgInfo.name}`) if (!test.checkPackageInPackageJSON(parentPkgJSON.name, dependentPkgJSON)) { throw new Error(`${parentPkgInfo.owner}/${parentPkgJSON.name} not found in the package.json of ${dependentPkgInfo.owner}/${dependentPkgInfo.name}`) @@ -23,9 +24,9 @@ module.exports = async function ({ dependents }) { } const runs = resp.data.check_runs || resp.data - console.log(`Tests on branch "${branch}"`) + debug(`Tests on branch "${branch}"`) const results = getResultForEachRun(runs) - console.log(results) + debug(results) } } diff --git a/lib/test.js b/lib/test.js index 81d3823..f2b513e 100644 --- a/lib/test.js +++ b/lib/test.js @@ -1,19 +1,27 @@ const fsPromises = require('fs').promises const github = require('./github') const gitURLParse = require('git-url-parse') +const logger = require('./logger') + +// setup logger namespace +const testCommandNamespace = 'wiby:test' +const debug = logger(testCommandNamespace) module.exports = async function ({ dependents }) { + // enable log output for test command without DEBUG env + logger.enableLogs(testCommandNamespace) + const parentPkgJSON = await getLocalPackageJSON() const parentPkgInfo = gitURLParse(parentPkgJSON.repository.url) - console.log(`Parent module: ${parentPkgInfo.owner}/${parentPkgJSON.name}`) + debug(`Parent module: ${parentPkgInfo.owner}/${parentPkgJSON.name}`) const commitURL = await getCommitURL(parentPkgInfo.owner, parentPkgInfo.name) - console.log('Commit URL to test:', commitURL) + debug('Commit URL to test:', commitURL) for (const { repository: url } of dependents) { const dependentPkgInfo = gitURLParse(url) const dependentPkgJSON = await github.getPackageJson(dependentPkgInfo.owner, dependentPkgInfo.name) - console.log(`Dependent module: ${dependentPkgInfo.owner}/${dependentPkgInfo.name}`) + debug(`Dependent module: ${dependentPkgInfo.owner}/${dependentPkgInfo.name}`) if (!checkPackageInPackageJSON(parentPkgJSON.name, dependentPkgJSON)) { throw new Error(`${parentPkgInfo.owner}/${parentPkgJSON.name} not found in the package.json of ${dependentPkgInfo.owner}/${dependentPkgInfo.name}`) @@ -68,5 +76,5 @@ async function pushPatch (packageJSON, owner, repo, dep) { const newTreeSha = await github.createTree(owner, repo, treeSha, blobSha) const commitSha = await github.createCommit(owner, repo, message, newTreeSha, headSha) await github.createBranch(owner, repo, commitSha, branch) - console.log(`Changes pushed to https://github.com/${owner}/${repo}/blob/${branch}/package.json`) + debug(`Changes pushed to https://github.com/${owner}/${repo}/blob/${branch}/package.json`) } diff --git a/package.json b/package.json index c24fb02..b898539 100644 --- a/package.json +++ b/package.json @@ -30,6 +30,7 @@ "@octokit/graphql": "^4.5.0", "@octokit/rest": "^18.0.0", "chalk": "^4.1.0", + "debug": "^4.3.1", "dotenv": "^8.2.0", "git-url-parse": "^11.1.2", "joi": "^17.2.1", diff --git a/test/fixtures/http/result-command-empty-branch-checks.js b/test/fixtures/http/result-command-empty-branch-checks.js index 9700874..372fa96 100644 --- a/test/fixtures/http/result-command-empty-branch-checks.js +++ b/test/fixtures/http/result-command-empty-branch-checks.js @@ -2,6 +2,7 @@ * Mocks of HTTP calls for "wiby result" command flow with empty response from check-runs */ const nock = require('nock') +require('../../../lib/logger').enableLogs() nock('https://api.github.com') // get package json diff --git a/test/fixtures/http/result-command-positive.js b/test/fixtures/http/result-command-positive.js index 233b7b3..8f17194 100644 --- a/test/fixtures/http/result-command-positive.js +++ b/test/fixtures/http/result-command-positive.js @@ -2,6 +2,7 @@ * Mocks of HTTP calls for "wiby result" command positive flow */ const nock = require('nock') +require('../../../lib/logger').enableLogs() nock('https://api.github.com') // get package json diff --git a/test/fixtures/http/result-command-wrong-package-json.js b/test/fixtures/http/result-command-wrong-package-json.js index 6675d8c..7105cfb 100644 --- a/test/fixtures/http/result-command-wrong-package-json.js +++ b/test/fixtures/http/result-command-wrong-package-json.js @@ -2,6 +2,7 @@ * Mocks of HTTP calls for "wiby result" command flow with package.json without expected dependency */ const nock = require('nock') +require('../../../lib/logger').enableLogs() nock('https://api.github.com') // get package json diff --git a/test/fixtures/http/test-command-positive.js b/test/fixtures/http/test-command-positive.js index b66dee5..1e020f2 100644 --- a/test/fixtures/http/test-command-positive.js +++ b/test/fixtures/http/test-command-positive.js @@ -2,6 +2,7 @@ * Mocks of HTTP calls for "wiby test" command positive flow */ const nock = require('nock') +require('../../../lib/logger').enableLogs() function nockPkgjsWiby (nockInstance) { return nockInstance diff --git a/test/logger.js b/test/logger.js new file mode 100644 index 0000000..76bd620 --- /dev/null +++ b/test/logger.js @@ -0,0 +1,31 @@ +const tap = require('tap') +const logger = require('./../lib/logger') + +tap.test('logger module', async (tap) => { + tap.test('logger should have default params', async (tap) => { + const debug = logger() + + tap.equals('wiby:general', debug.namespace) + tap.equals(true, Object.prototype.hasOwnProperty.call(debug, 'log')) + }) + + tap.test('logger should assign passed namespace', async (tap) => { + const debug = logger('wiby:unit-test') + + tap.equals('wiby:unit-test', debug.namespace) + }) + + tap.test('logger should not change default output stream in case writeToStdOut = false', async (tap) => { + const debug = logger('wiby:unit-test', false) + + tap.equals('wiby:unit-test', debug.namespace) + tap.equals(false, Object.prototype.hasOwnProperty.call(debug, 'log')) + }) + + tap.test('module should have enableLogs with default param', async (tap) => { + const debug = logger() + logger.enableLogs() + + tap.equals(true, debug.enabled) + }) +}) From ea3ca7c639bed827a52293faf972799519e28c6c Mon Sep 17 00:00:00 2001 From: Rodion Abdurakhimov Date: Fri, 8 Jan 2021 23:34:58 +0200 Subject: [PATCH 2/5] feat(result): Make default result export return data object with results --- bin/commands/result.js | 4 ++-- lib/result.js | 52 ++++++++++++++++++++++++++++++++++++++++++ test/result.js | 50 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 2 deletions(-) diff --git a/bin/commands/result.js b/bin/commands/result.js index 8c81d76..e07a747 100644 --- a/bin/commands/result.js +++ b/bin/commands/result.js @@ -15,10 +15,10 @@ exports.builder = (yargs) => yargs type: 'string' }) -exports.handler = (params) => { +exports.handler = async (params) => { const config = params.dependent ? { dependents: [{ repository: params.dependent }] } : wiby.validate({ config: params.config }) - return wiby.result(config) + return await wiby.result(config) } diff --git a/lib/result.js b/lib/result.js index 347539f..e401ceb 100644 --- a/lib/result.js +++ b/lib/result.js @@ -6,6 +6,9 @@ const debug = require('./logger')('wiby:result') module.exports = async function ({ dependents }) { const parentPkgJSON = await test.getLocalPackageJSON() const parentPkgInfo = gitURLParse(parentPkgJSON.repository.url) + const output = { status: 'pending', results: [] } + const allDependantsChecks = [] + debug(`Parent module: ${parentPkgInfo.owner}/${parentPkgJSON.name}`) for (const { repository: url } of dependents) { @@ -26,8 +29,23 @@ module.exports = async function ({ dependents }) { const runs = resp.data.check_runs || resp.data debug(`Tests on branch "${branch}"`) const results = getResultForEachRun(runs) + + // add current dependent check to general list for calculating overall status + results.forEach(runResult => (allDependantsChecks.push(runResult))) + + // form a result object with dependents statuses + output.results.push({ + dependent: `${dependentPkgInfo.owner}/${dependentPkgInfo.name}`, + status: getOverallStatusForAllRuns(results), + runs: results + }) + debug(results) } + + output.status = getOverallStatusForAllRuns(allDependantsChecks) + + return output } const getBranchName = module.exports.getBranchName = async function getBranchName (dep) { @@ -42,3 +60,37 @@ const getResultForEachRun = module.exports.getResultForEachRun = function getRes return [check.name, check.conclusion] }) } + +/** + * Accepts pipeline checks results and resolves overall checks status for passed array of runs + * Retunts 'failure', 'pending' or 'success' + */ +const getOverallStatusForAllRuns = module.exports.getOverallStatusForAllRuns = function getOverallStatusForAllRuns (runs) { + const statuses = [] + let isAllSuccess = true + + runs.forEach(runResult => { + // run get status + const status = runResult[1] + + if (status !== 'success') { + isAllSuccess = false + } + + statuses.push(status) + }) + + // if includes at least 1 failure - overall status is failure + if (statuses.includes('failure')) { + return 'failure' + } + + // if includes null or pending - overall status us pending + if (statuses.includes(null) || statuses.includes('pending')) { + return 'pending' + } + + // return success only in case all result = 'success' + // return failure in case unexpected status detected + return isAllSuccess ? 'success' : 'failure' +} diff --git a/test/result.js b/test/result.js index 0dfa4ff..b5355e0 100644 --- a/test/result.js +++ b/test/result.js @@ -47,3 +47,53 @@ tap.test('result command checks package exists in dependant package.json', tap = ) tap.end() }) + +tap.test('result() should return correct data object', async tap => { + // mock reall http requests with positive scenario + require('./fixtures/http/result-command-positive') + + const output = await result({ dependents: [{ repository: 'https://github.com/wiby-test/fakeRepo' }] }) + + tap.equal(output.status, 'failure') + tap.equal(output.results[0].dependent, 'wiby-test/fakeRepo') + tap.equal(output.results[0].status, 'failure') + tap.equal(output.results[0].runs.length, 2) +}) + +tap.test('should correctly detect overall status for runs results', async tap => { + const failedCasesPresent = [ + [null, 'failure'], + [null, 'success'], + [null, 'success'] + ] + + const unexpectedCasesPresent = [ + [null, 'unexpectedStatus'], + [null, 'success'], + [null, 'success'] + ] + + const nullCasesPresent = [ + [null, null], + [null, 'success'], + [null, 'success'] + ] + + const pendingCasesPresent = [ + [null, 'pending'], + [null, 'success'], + [null, 'success'] + ] + + const successCasesPresent = [ + [null, 'success'], + [null, 'success'], + [null, 'success'] + ] + + tap.equal(result.getOverallStatusForAllRuns(failedCasesPresent), 'failure') + tap.equal(result.getOverallStatusForAllRuns(unexpectedCasesPresent), 'failure') + tap.equal(result.getOverallStatusForAllRuns(nullCasesPresent), 'pending') + tap.equal(result.getOverallStatusForAllRuns(pendingCasesPresent), 'pending') + tap.equal(result.getOverallStatusForAllRuns(successCasesPresent), 'success') +}) From 9a87223f640c4cdd872955a90868760fdbbbc6c9 Mon Sep 17 00:00:00 2001 From: Rodion Abdurakhimov Date: Sat, 9 Jan 2021 16:16:54 +0200 Subject: [PATCH 3/5] feat(result): output as markdown and new exit codes --- bin/commands/result.js | 4 +- lib/result.js | 89 ++++++++++++-- test/cli.js | 112 +++++++++++++----- .../result-output-multiple-dependant.md | 24 ++++ ...lt-output-single-dependant-check-failed.md | 9 ++ .../result/result-output-single-dependant.md | 10 ++ .../result-command-empty-branch-checks.js | 1 - .../result-command-positive-checks-failed.js | 30 +++++ test/fixtures/http/result-command-positive.js | 1 - .../http/result-command-wrong-package-json.js | 1 - test/fixtures/http/test-command-positive.js | 1 - test/logger.js | 4 +- test/result.js | 13 +- 13 files changed, 246 insertions(+), 53 deletions(-) create mode 100644 test/fixtures/expected-outputs/result/result-output-multiple-dependant.md create mode 100644 test/fixtures/expected-outputs/result/result-output-single-dependant-check-failed.md create mode 100644 test/fixtures/expected-outputs/result/result-output-single-dependant.md create mode 100644 test/fixtures/http/result-command-positive-checks-failed.js diff --git a/bin/commands/result.js b/bin/commands/result.js index e07a747..01f7fc1 100644 --- a/bin/commands/result.js +++ b/bin/commands/result.js @@ -20,5 +20,7 @@ exports.handler = async (params) => { ? { dependents: [{ repository: params.dependent }] } : wiby.validate({ config: params.config }) - return await wiby.result(config) + const result = await wiby.result(config) + + return wiby.result.processOutput(result) } diff --git a/lib/result.js b/lib/result.js index e401ceb..501b832 100644 --- a/lib/result.js +++ b/lib/result.js @@ -3,11 +3,21 @@ const github = require('./github') const gitURLParse = require('git-url-parse') const debug = require('./logger')('wiby:result') +// enum containing possible pipeline checks statuses +const pipelineStatsesEnum = module.exports.pipelineStatsesEnum = Object.freeze({ + FAILED: 'failure', + QUEUED: 'queued', + PENDING: 'pending', + SUCCEED: 'success' +}) + +const PENDING_RESULT_EXIT_CODE = 64 + module.exports = async function ({ dependents }) { const parentPkgJSON = await test.getLocalPackageJSON() const parentPkgInfo = gitURLParse(parentPkgJSON.repository.url) - const output = { status: 'pending', results: [] } - const allDependantsChecks = [] + const output = { status: pipelineStatsesEnum.PENDING, results: [] } + const allDependentsChecks = [] debug(`Parent module: ${parentPkgInfo.owner}/${parentPkgJSON.name}`) @@ -31,7 +41,7 @@ module.exports = async function ({ dependents }) { const results = getResultForEachRun(runs) // add current dependent check to general list for calculating overall status - results.forEach(runResult => (allDependantsChecks.push(runResult))) + results.forEach(runResult => (allDependentsChecks.push(runResult))) // form a result object with dependents statuses output.results.push({ @@ -43,7 +53,9 @@ module.exports = async function ({ dependents }) { debug(results) } - output.status = getOverallStatusForAllRuns(allDependantsChecks) + output.status = getOverallStatusForAllRuns(allDependentsChecks) + + debug(output) return output } @@ -54,7 +66,7 @@ const getBranchName = module.exports.getBranchName = async function getBranchNam const getResultForEachRun = module.exports.getResultForEachRun = function getResultForEachRun (runs) { return runs.map((check) => { - if (check.status === 'queued') { + if (check.status === pipelineStatsesEnum.QUEUED) { return [check.name, check.status] } return [check.name, check.conclusion] @@ -63,7 +75,7 @@ const getResultForEachRun = module.exports.getResultForEachRun = function getRes /** * Accepts pipeline checks results and resolves overall checks status for passed array of runs - * Retunts 'failure', 'pending' or 'success' + * Returns 'failure', 'pending' or 'success' */ const getOverallStatusForAllRuns = module.exports.getOverallStatusForAllRuns = function getOverallStatusForAllRuns (runs) { const statuses = [] @@ -73,7 +85,7 @@ const getOverallStatusForAllRuns = module.exports.getOverallStatusForAllRuns = f // run get status const status = runResult[1] - if (status !== 'success') { + if (status !== pipelineStatsesEnum.SUCCEED) { isAllSuccess = false } @@ -81,16 +93,67 @@ const getOverallStatusForAllRuns = module.exports.getOverallStatusForAllRuns = f }) // if includes at least 1 failure - overall status is failure - if (statuses.includes('failure')) { - return 'failure' + if (statuses.includes(pipelineStatsesEnum.FAILED)) { + return pipelineStatsesEnum.FAILED } - // if includes null or pending - overall status us pending - if (statuses.includes(null) || statuses.includes('pending')) { - return 'pending' + // if includes null or pending or queued - overall status is pending + if (statuses.includes(null) || + statuses.includes(pipelineStatsesEnum.PENDING) || + statuses.includes(pipelineStatsesEnum.QUEUED) + ) { + return pipelineStatsesEnum.PENDING } // return success only in case all result = 'success' // return failure in case unexpected status detected - return isAllSuccess ? 'success' : 'failure' + return isAllSuccess ? pipelineStatsesEnum.SUCCEED : pipelineStatsesEnum.FAILED +} + +/** + * Accepts result object from wiby.result(), outputs markdown + * and resolves exit code based on overall status + */ +module.exports.processOutput = function processOutput (result) { + printResultOutputMd(result) + resolveCodeAndExit(result.status) +} + +/** + * Builds and outputs markdown for wiby.result() + */ +function printResultOutputMd (result) { + let md = `# wiby result command + +Overall status - ${result.status}\n\n` + + result.results.forEach(dependent => { + md += `## ${dependent.dependent} - ${dependent.status} + +Checks:\n\n` + + dependent.runs.forEach((run, index, arr) => { + md += `- ${run[0]} - ${run[1]}\n` + + // extra line for next dependent + if (arr.length - 1 === index) { + md += '\n' + } + }) + }) + + // output built markdown + console.log(md) +} + +/** + * Based on overall status from wiby.result() - resolves with what exit code + * process should be ended + */ +function resolveCodeAndExit (overallStatus) { + if (overallStatus === pipelineStatsesEnum.FAILED) { + process.exit(1) + } else if (overallStatus === pipelineStatsesEnum.PENDING) { + process.exit(PENDING_RESULT_EXIT_CODE) + } } diff --git a/test/cli.js b/test/cli.js index 27f4ea1..6f2c26a 100644 --- a/test/cli.js +++ b/test/cli.js @@ -1,10 +1,13 @@ const tap = require('tap') const childProcess = require('child_process') const path = require('path') +const fs = require('fs') const wibyCommand = path.join(__dirname, '..', 'bin', 'wiby') const cwd = path.join(__dirname, '..') +const PENDING_RESULT_EXIT_CODE = 64 + tap.test('test command', async (tap) => { tap.test('test command should fail when config and dependent provided', async (tap) => { try { @@ -51,47 +54,94 @@ tap.test('result command', async (tap) => { }) tap.test('result command should call result module with dependent URI', async (tap) => { - const result = childProcess.execSync(`${wibyCommand} result --dependent="https://github.com/wiby-test/fakeRepo"`, { - cwd: cwd, - env: { - NODE_OPTIONS: '-r ./test/fixtures/http/result-command-positive.js' - } - }).toString() + const expected = fs.readFileSync( + path.join(__dirname, '.', 'fixtures', 'expected-outputs', 'result', 'result-output-single-dependant.md'), + 'utf-8' + ) + .trim() - tap.equal(true, result.includes("[ 'fake_run', 'queued' ]")) - tap.equal(true, result.includes("[ 'fake_run_2', 'fake_conclusion' ]")) + try { + childProcess.execSync(`${wibyCommand} result --dependent="https://github.com/wiby-test/fakeRepo"`, { + cwd: cwd, + env: { + NODE_OPTIONS: '-r ./test/fixtures/http/result-command-positive.js' + } + }) + } catch (e) { + const result = e.output[1].toString().trim() + + tap.equal(result, expected) + tap.equal(e.status, PENDING_RESULT_EXIT_CODE) + } }) - tap.test('test command should call test module with all deps from .wiby.json', async (tap) => { - const result = childProcess.execSync(`${wibyCommand} result`, { - cwd: cwd, - env: { - NODE_OPTIONS: '-r ./test/fixtures/http/result-command-positive.js' - } - }).toString() - - tap.equal(true, result.includes("[ 'fail_run', 'queued' ]")) - tap.equal(true, result.includes("[ 'fail_run_2', 'fake_conclusion' ]")) - - tap.equal(true, result.includes("[ 'pass_run', 'queued' ]")) - tap.equal(true, result.includes("[ 'pass_run_2', 'fake_conclusion' ]")) + tap.test('result command should call result module with all deps from .wiby.json', async (tap) => { + const expected = fs.readFileSync( + path.join(__dirname, '.', 'fixtures', 'expected-outputs', 'result', 'result-output-multiple-dependant.md'), + 'utf-8' + ) + .trim() - tap.equal(true, result.includes("[ 'partial_run', 'queued' ]")) - tap.equal(true, result.includes("[ 'partial_run_2', 'fake_conclusion' ]")) + try { + childProcess.execSync(`${wibyCommand} result`, { + cwd: cwd, + env: { + NODE_OPTIONS: '-r ./test/fixtures/http/result-command-positive.js' + } + }) + } catch (e) { + const result = e.output[1].toString().trim() + + tap.equal(result, expected) + tap.equal(e.status, PENDING_RESULT_EXIT_CODE) + } }) tap.test('result command handles empty response from github.getChecks()', tap => { - const result = childProcess.execSync(`${wibyCommand} result --dependent="https://github.com/wiby-test/fakeRepo"`, { - cwd: cwd, - env: { - NODE_OPTIONS: '-r ./test/fixtures/http/result-command-empty-branch-checks.js' - } - }).toString() + const expected = fs.readFileSync( + path.join(__dirname, '.', 'fixtures', 'expected-outputs', 'result', 'result-output-single-dependant.md'), + 'utf-8' + ) + .trim() + + try { + childProcess.execSync(`${wibyCommand} result --dependent="https://github.com/wiby-test/fakeRepo"`, { + cwd: cwd, + env: { + NODE_OPTIONS: '-r ./test/fixtures/http/result-command-empty-branch-checks.js' + } + }) + } catch (e) { + const result = e.output[1].toString().trim() + + tap.equal(result, expected) + tap.equal(e.status, PENDING_RESULT_EXIT_CODE) + } - tap.equal(true, result.includes("[ 'fake_run', 'queued' ]")) - tap.equal(true, result.includes("[ 'fake_run_2', 'fake_conclusion' ]")) tap.end() }) + + tap.test('result command should exit with code 1 for overall failed status', async (tap) => { + const expected = fs.readFileSync( + path.join(__dirname, '.', 'fixtures', 'expected-outputs', 'result', 'result-output-single-dependant-check-failed.md'), + 'utf-8' + ) + .trim() + + try { + childProcess.execSync(`${wibyCommand} result --dependent="https://github.com/wiby-test/fakeRepo"`, { + cwd: cwd, + env: { + NODE_OPTIONS: '-r ./test/fixtures/http/result-command-positive-checks-failed.js' + } + }) + } catch (e) { + const result = e.output[1].toString().trim() + + tap.equal(result, expected) + tap.equal(e.status, 1) + } + }) }) tap.test('validate command', async (tap) => { diff --git a/test/fixtures/expected-outputs/result/result-output-multiple-dependant.md b/test/fixtures/expected-outputs/result/result-output-multiple-dependant.md new file mode 100644 index 0000000..875f62f --- /dev/null +++ b/test/fixtures/expected-outputs/result/result-output-multiple-dependant.md @@ -0,0 +1,24 @@ +# wiby result command + +Overall status - pending + +## wiby-test/partial - pending + +Checks: + +- partial_run - queued +- partial_run_2 - fake_conclusion + +## wiby-test/fail - pending + +Checks: + +- fail_run - queued +- fail_run_2 - fake_conclusion + +## wiby-test/pass - pending + +Checks: + +- pass_run - queued +- pass_run_2 - fake_conclusion diff --git a/test/fixtures/expected-outputs/result/result-output-single-dependant-check-failed.md b/test/fixtures/expected-outputs/result/result-output-single-dependant-check-failed.md new file mode 100644 index 0000000..c3790c1 --- /dev/null +++ b/test/fixtures/expected-outputs/result/result-output-single-dependant-check-failed.md @@ -0,0 +1,9 @@ +# wiby result command + +Overall status - failure + +## wiby-test/fakeRepo - failure + +Checks: + +- fake_run - failure diff --git a/test/fixtures/expected-outputs/result/result-output-single-dependant.md b/test/fixtures/expected-outputs/result/result-output-single-dependant.md new file mode 100644 index 0000000..b91f4a3 --- /dev/null +++ b/test/fixtures/expected-outputs/result/result-output-single-dependant.md @@ -0,0 +1,10 @@ +# wiby result command + +Overall status - pending + +## wiby-test/fakeRepo - pending + +Checks: + +- fake_run - queued +- fake_run_2 - fake_conclusion diff --git a/test/fixtures/http/result-command-empty-branch-checks.js b/test/fixtures/http/result-command-empty-branch-checks.js index 372fa96..9700874 100644 --- a/test/fixtures/http/result-command-empty-branch-checks.js +++ b/test/fixtures/http/result-command-empty-branch-checks.js @@ -2,7 +2,6 @@ * Mocks of HTTP calls for "wiby result" command flow with empty response from check-runs */ const nock = require('nock') -require('../../../lib/logger').enableLogs() nock('https://api.github.com') // get package json diff --git a/test/fixtures/http/result-command-positive-checks-failed.js b/test/fixtures/http/result-command-positive-checks-failed.js new file mode 100644 index 0000000..1bec118 --- /dev/null +++ b/test/fixtures/http/result-command-positive-checks-failed.js @@ -0,0 +1,30 @@ +/** + * Mocks of HTTP calls for "wiby result" command + * Checks returned with status failure + */ +const nock = require('nock') + +nock('https://api.github.com') + // get package json + .post('/graphql') + .times(3) + .reply(200, { + data: { + repository: { + object: { + text: JSON.stringify({ + dependencies: { + wiby: '*' + } + }) + } + } + } + }) + // get check results + .get('/repos/wiby-test/fakeRepo/commits/wiby-wiby/check-runs') + .reply(200, { + check_runs: [ + { status: 'done', name: 'fake_run', conclusion: 'failure' } + ] + }) diff --git a/test/fixtures/http/result-command-positive.js b/test/fixtures/http/result-command-positive.js index 8f17194..233b7b3 100644 --- a/test/fixtures/http/result-command-positive.js +++ b/test/fixtures/http/result-command-positive.js @@ -2,7 +2,6 @@ * Mocks of HTTP calls for "wiby result" command positive flow */ const nock = require('nock') -require('../../../lib/logger').enableLogs() nock('https://api.github.com') // get package json diff --git a/test/fixtures/http/result-command-wrong-package-json.js b/test/fixtures/http/result-command-wrong-package-json.js index 7105cfb..6675d8c 100644 --- a/test/fixtures/http/result-command-wrong-package-json.js +++ b/test/fixtures/http/result-command-wrong-package-json.js @@ -2,7 +2,6 @@ * Mocks of HTTP calls for "wiby result" command flow with package.json without expected dependency */ const nock = require('nock') -require('../../../lib/logger').enableLogs() nock('https://api.github.com') // get package json diff --git a/test/fixtures/http/test-command-positive.js b/test/fixtures/http/test-command-positive.js index 1e020f2..b66dee5 100644 --- a/test/fixtures/http/test-command-positive.js +++ b/test/fixtures/http/test-command-positive.js @@ -2,7 +2,6 @@ * Mocks of HTTP calls for "wiby test" command positive flow */ const nock = require('nock') -require('../../../lib/logger').enableLogs() function nockPkgjsWiby (nockInstance) { return nockInstance diff --git a/test/logger.js b/test/logger.js index 76bd620..7bff53b 100644 --- a/test/logger.js +++ b/test/logger.js @@ -6,6 +6,7 @@ tap.test('logger module', async (tap) => { const debug = logger() tap.equals('wiby:general', debug.namespace) + // check that output stream changed by default by overriding "log" method tap.equals(true, Object.prototype.hasOwnProperty.call(debug, 'log')) }) @@ -19,10 +20,11 @@ tap.test('logger module', async (tap) => { const debug = logger('wiby:unit-test', false) tap.equals('wiby:unit-test', debug.namespace) + // check that default output stream not changed tap.equals(false, Object.prototype.hasOwnProperty.call(debug, 'log')) }) - tap.test('module should have enableLogs with default param', async (tap) => { + tap.test('module should have enableLogs() with default params', async (tap) => { const debug = logger() logger.enableLogs() diff --git a/test/result.js b/test/result.js index b5355e0..4c61225 100644 --- a/test/result.js +++ b/test/result.js @@ -54,13 +54,13 @@ tap.test('result() should return correct data object', async tap => { const output = await result({ dependents: [{ repository: 'https://github.com/wiby-test/fakeRepo' }] }) - tap.equal(output.status, 'failure') + tap.equal(output.status, 'pending') tap.equal(output.results[0].dependent, 'wiby-test/fakeRepo') - tap.equal(output.results[0].status, 'failure') + tap.equal(output.results[0].status, 'pending') tap.equal(output.results[0].runs.length, 2) }) -tap.test('should correctly detect overall status for runs results', async tap => { +tap.test('getOverallStatusForAllRuns() should correctly detect overall status for runs results', async tap => { const failedCasesPresent = [ [null, 'failure'], [null, 'success'], @@ -85,6 +85,12 @@ tap.test('should correctly detect overall status for runs results', async tap => [null, 'success'] ] + const queuedCasesPresent = [ + [null, 'queued'], + [null, 'success'], + [null, 'success'] + ] + const successCasesPresent = [ [null, 'success'], [null, 'success'], @@ -95,5 +101,6 @@ tap.test('should correctly detect overall status for runs results', async tap => tap.equal(result.getOverallStatusForAllRuns(unexpectedCasesPresent), 'failure') tap.equal(result.getOverallStatusForAllRuns(nullCasesPresent), 'pending') tap.equal(result.getOverallStatusForAllRuns(pendingCasesPresent), 'pending') + tap.equal(result.getOverallStatusForAllRuns(queuedCasesPresent), 'pending') tap.equal(result.getOverallStatusForAllRuns(successCasesPresent), 'success') }) From f1a8cd7d00b0ea290ed24c2b7c5be2398ce3daee Mon Sep 17 00:00:00 2001 From: Rodion Abdurakhimov Date: Wed, 13 Jan 2021 21:47:24 +0200 Subject: [PATCH 4/5] refactor(result): simplify "result.getOverallStatusForAllRuns()" return statements --- lib/result.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/result.js b/lib/result.js index 501b832..64b5915 100644 --- a/lib/result.js +++ b/lib/result.js @@ -92,6 +92,11 @@ const getOverallStatusForAllRuns = module.exports.getOverallStatusForAllRuns = f statuses.push(status) }) + // return success only in case all result = 'success' + if (isAllSuccess) { + return pipelineStatsesEnum.SUCCEED + } + // if includes at least 1 failure - overall status is failure if (statuses.includes(pipelineStatsesEnum.FAILED)) { return pipelineStatsesEnum.FAILED @@ -105,9 +110,8 @@ const getOverallStatusForAllRuns = module.exports.getOverallStatusForAllRuns = f return pipelineStatsesEnum.PENDING } - // return success only in case all result = 'success' // return failure in case unexpected status detected - return isAllSuccess ? pipelineStatsesEnum.SUCCEED : pipelineStatsesEnum.FAILED + return pipelineStatsesEnum.FAILED } /** From fb9445983fe1606d73ed71030e82b057ac30e29c Mon Sep 17 00:00:00 2001 From: Rodion Abdurakhimov Date: Mon, 18 Jan 2021 18:40:33 +0200 Subject: [PATCH 5/5] fix(result): typo in pipelineStatusesEnum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dominykas Blyžė --- lib/result.js | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/result.js b/lib/result.js index 64b5915..5ed9e21 100644 --- a/lib/result.js +++ b/lib/result.js @@ -4,7 +4,7 @@ const gitURLParse = require('git-url-parse') const debug = require('./logger')('wiby:result') // enum containing possible pipeline checks statuses -const pipelineStatsesEnum = module.exports.pipelineStatsesEnum = Object.freeze({ +const pipelineStatusesEnum = module.exports.pipelineStatusesEnum = Object.freeze({ FAILED: 'failure', QUEUED: 'queued', PENDING: 'pending', @@ -16,7 +16,7 @@ const PENDING_RESULT_EXIT_CODE = 64 module.exports = async function ({ dependents }) { const parentPkgJSON = await test.getLocalPackageJSON() const parentPkgInfo = gitURLParse(parentPkgJSON.repository.url) - const output = { status: pipelineStatsesEnum.PENDING, results: [] } + const output = { status: pipelineStatusesEnum.PENDING, results: [] } const allDependentsChecks = [] debug(`Parent module: ${parentPkgInfo.owner}/${parentPkgJSON.name}`) @@ -66,7 +66,7 @@ const getBranchName = module.exports.getBranchName = async function getBranchNam const getResultForEachRun = module.exports.getResultForEachRun = function getResultForEachRun (runs) { return runs.map((check) => { - if (check.status === pipelineStatsesEnum.QUEUED) { + if (check.status === pipelineStatusesEnum.QUEUED) { return [check.name, check.status] } return [check.name, check.conclusion] @@ -85,7 +85,7 @@ const getOverallStatusForAllRuns = module.exports.getOverallStatusForAllRuns = f // run get status const status = runResult[1] - if (status !== pipelineStatsesEnum.SUCCEED) { + if (status !== pipelineStatusesEnum.SUCCEED) { isAllSuccess = false } @@ -94,24 +94,24 @@ const getOverallStatusForAllRuns = module.exports.getOverallStatusForAllRuns = f // return success only in case all result = 'success' if (isAllSuccess) { - return pipelineStatsesEnum.SUCCEED + return pipelineStatusesEnum.SUCCEED } // if includes at least 1 failure - overall status is failure - if (statuses.includes(pipelineStatsesEnum.FAILED)) { - return pipelineStatsesEnum.FAILED + if (statuses.includes(pipelineStatusesEnum.FAILED)) { + return pipelineStatusesEnum.FAILED } // if includes null or pending or queued - overall status is pending if (statuses.includes(null) || - statuses.includes(pipelineStatsesEnum.PENDING) || - statuses.includes(pipelineStatsesEnum.QUEUED) + statuses.includes(pipelineStatusesEnum.PENDING) || + statuses.includes(pipelineStatusesEnum.QUEUED) ) { - return pipelineStatsesEnum.PENDING + return pipelineStatusesEnum.PENDING } // return failure in case unexpected status detected - return pipelineStatsesEnum.FAILED + return pipelineStatusesEnum.FAILED } /** @@ -155,9 +155,9 @@ Checks:\n\n` * process should be ended */ function resolveCodeAndExit (overallStatus) { - if (overallStatus === pipelineStatsesEnum.FAILED) { + if (overallStatus === pipelineStatusesEnum.FAILED) { process.exit(1) - } else if (overallStatus === pipelineStatsesEnum.PENDING) { + } else if (overallStatus === pipelineStatusesEnum.PENDING) { process.exit(PENDING_RESULT_EXIT_CODE) } }