Skip to content
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

feat(wiby result): change result command output and exit codes #74

Merged
merged 5 commits into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 4 additions & 2 deletions bin/commands/result.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ 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)
const result = await wiby.result(config)

return wiby.result.processOutput(result)
}
3 changes: 2 additions & 1 deletion lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -32,7 +33,7 @@ exports.validate = ({ config: configFilePath }) => {
throw result.error
}

console.log(`✅ ${configFilePath}`)
debug(`✅ ${configFilePath}`)

return result.value
}
26 changes: 26 additions & 0 deletions lib/logger.js
Original file line number Diff line number Diff line change
@@ -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) {
rodion-arr marked this conversation as resolved.
Show resolved Hide resolved
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)
}
130 changes: 125 additions & 5 deletions lib/result.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,30 @@
const test = require('./test')
const github = require('./github')
const gitURLParse = require('git-url-parse')
const debug = require('./logger')('wiby:result')

// enum containing possible pipeline checks statuses
const pipelineStatusesEnum = module.exports.pipelineStatusesEnum = 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)
console.log(`Parent module: ${parentPkgInfo.owner}/${parentPkgJSON.name}`)
const output = { status: pipelineStatusesEnum.PENDING, results: [] }
const allDependentsChecks = []

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}`)
Expand All @@ -23,10 +37,27 @@ 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)

// add current dependent check to general list for calculating overall status
results.forEach(runResult => (allDependentsChecks.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(allDependentsChecks)

debug(output)

return output
}

const getBranchName = module.exports.getBranchName = async function getBranchName (dep) {
Expand All @@ -35,9 +66,98 @@ 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 === pipelineStatusesEnum.QUEUED) {
return [check.name, check.status]
}
return [check.name, check.conclusion]
})
}

/**
* Accepts pipeline checks results and resolves overall checks status for passed array of runs
* Returns '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 !== pipelineStatusesEnum.SUCCEED) {
isAllSuccess = false
}

statuses.push(status)
})

Copy link
Contributor

Choose a reason for hiding this comment

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

If I am reading this correctly once we have completed iterating over all the runs and the isAllSuccess is still true.

Then following the methodology below should we return pipelineStatses.SUCCEED and change the final return to pipelineStatses.FAILED ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed this will simplify code a bit, submitted changes

// return success only in case all result = 'success'
if (isAllSuccess) {
return pipelineStatusesEnum.SUCCEED
}

// if includes at least 1 failure - overall status is failure
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(pipelineStatusesEnum.PENDING) ||
statuses.includes(pipelineStatusesEnum.QUEUED)
) {
return pipelineStatusesEnum.PENDING
}

// return failure in case unexpected status detected
return pipelineStatusesEnum.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 === pipelineStatusesEnum.FAILED) {
process.exit(1)
} else if (overallStatus === pipelineStatusesEnum.PENDING) {
process.exit(PENDING_RESULT_EXIT_CODE)
}
}
16 changes: 12 additions & 4 deletions lib/test.js
Original file line number Diff line number Diff line change
@@ -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}`)
Expand Down Expand Up @@ -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`)
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading