Repository: infinitered/solidarity
Pull request #89 (2017-10-22 13:52:52)

Modified output and added options as discussed in #85
Github issue #85

I wasn't sure on the following things:
- If the `"config" { "output" : "value" }` tree is redundant and should the `output` be on the top level of the config
- If how the output works should be changed (printing a final message even on `silent`)
- The documentation is in the correct place

Let me know what you think!

codecov[bot]: # [Codecov](https://codecov.io/gh/infinitered/solidarity/pull/89?src=pr&el=h1) Report
> Merging [#89](https://codecov.io/gh/infinitered/solidarity/pull/89?src=pr&el=desc) into [master](https://codecov.io/gh/infinitered/solidarity/commit/4b7d31de1f36cbd53cee1f3ad2f9f2a93a20078f?src=pr&el=desc) will **decrease** coverage by `0.43%`.
> The diff coverage is `9.67%`.

[![Impacted file tree graph](https://codecov.io/gh/infinitered/solidarity/pull/89/graphs/tree.svg?width=650&height=150&src=pr&token=eTjcyCooFB)](https://codecov.io/gh/infinitered/solidarity/pull/89?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #89      +/-   ##
==========================================
- Coverage   31.06%   30.63%   -0.44%     
==========================================
  Files          20       20              
  Lines         309      333      +24     
  Branches       43       55      +12     
==========================================
+ Hits           96      102       +6     
- Misses        209      227      +18     
  Partials        4        4
```


| [Impacted Files](https://codecov.io/gh/infinitered/solidarity/pull/89?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/types.ts](https://codecov.io/gh/infinitered/solidarity/pull/89?src=pr&el=tree#diff-c3JjL3R5cGVzLnRz) | `100% <ø> (ø)` | :arrow_up: |
| [src/extensions/functions/checkCLI.ts](https://codecov.io/gh/infinitered/solidarity/pull/89?src=pr&el=tree#diff-c3JjL2V4dGVuc2lvbnMvZnVuY3Rpb25zL2NoZWNrQ0xJLnRz) | `80% <100%> (ø)` | :arrow_up: |
| [src/extensions/functions/checkRequirement.ts](https://codecov.io/gh/infinitered/solidarity/pull/89?src=pr&el=tree#diff-c3JjL2V4dGVuc2lvbnMvZnVuY3Rpb25zL2NoZWNrUmVxdWlyZW1lbnQudHM=) | `11.11% <5.55%> (-1.86%)` | :arrow_down: |
| [src/commands/solidarity.ts](https://codecov.io/gh/infinitered/solidarity/pull/89?src=pr&el=tree#diff-c3JjL2NvbW1hbmRzL3NvbGlkYXJpdHkudHM=) | `16.66% <8.33%> (-1.76%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/infinitered/solidarity/pull/89?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/infinitered/solidarity/pull/89?src=pr&el=footer). Last update [4b7d31d...171c749](https://codecov.io/gh/infinitered/solidarity/pull/89?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

GantMan: @Mknsri - great work!

Patch:
diff --git a/docs/options.md b/docs/options.md
index 74537cc..b08b0df 100644
--- a/docs/options.md
+++ b/docs/options.md
@@ -1,6 +1,21 @@
 # Solidarity Options
 Understanding the `.solidarity` file helps you read and write new solidarity checks for any project.
 
+## Solidarity output
+You can use set the following option in `.solidarity` to configure output:
+```
+{
+  "config" : {
+    "output" : "<moderate/verbose/silent>" // Default is moderate
+  }
+}
+```
+- Moderate - Only outputs message if a specific check fails
+- Verbose  - Outputs messages for successful and failed checks.
+- Silent   - Only outputs the final result for all checks (Pass/Fail)
+
+Optionally you can also pass `--verbose` in the CLI to override the configuration option and output all status messages.
+
 ## Solidarity Rules
 The `.solidarity` file is a JSON object with a set of requirements to enforce on each computer's environment.  All requirements should be specified inside the `requirements` key.
 
diff --git a/src/commands/solidarity.ts b/src/commands/solidarity.ts
index 2bcd75d..69d1d53 100644
--- a/src/commands/solidarity.ts
+++ b/src/commands/solidarity.ts
@@ -1,4 +1,5 @@
 import { GluegunCommand } from 'gluegun'
+import { SolidarityOutputMode } from '../types'
 
 namespace Solidarity {
   const { map, toPairs, isEmpty, flatten, reject, isNil } = require('ramda')
@@ -17,6 +18,17 @@ namespace Solidarity {
     }
   }
 
+  const setOutputMode = (parameters, settings) : SolidarityOutputMode => {
+    // CLI parameter --verbose overrides any config settings
+    if (parameters.options.verbose) {
+      return SolidarityOutputMode.VERBOSE
+    }
+
+    // Set output mode, set to default on invalid value
+    let outputModeString = settings.config ? String(settings.config.output).toUpperCase() : null
+    return SolidarityOutputMode[outputModeString] || SolidarityOutputMode.MODERATE
+  }
+
   export const run = async (context) => {
     // drop out fast in these situations
     checkForEscapeHatchFlags(context)
@@ -38,6 +50,8 @@ namespace Solidarity {
       process.exit(3)
     }
 
+    context.outputMode = setOutputMode(context.parameters, solidaritySettings)
+    
     // build map of checks to perform
     const checks = await map(
       async requirement => checkRequirement(requirement, context),
@@ -48,11 +62,18 @@ namespace Solidarity {
     await Promise.all(checks)
       .then(results => {
         const errors = reject(isNil, flatten(results))
+        const silentOutput = context.outputMode == SolidarityOutputMode.SILENT
+        // Add empty line between final result if printing rule results
+        if (!silentOutput) print.success('\n')
+        
         if (isEmpty(errors)) {
-          print.success('\n Environment Checks Valid')
+          print.success('Good to go.')
         } else {
-          print.error('\nSolidarity Checks Failed:\n')
-          print.error(errors.join('\n'))
+          print.error('Solidarity checks failed.')
+          // Print instructions in case silent logging is enabled
+          if (silentOutput) {
+            print.error('Change "output" in .solidarity-file or pass --verbose to see details.')
+          }
           process.exit(1)
         }
       })
diff --git a/src/extensions/functions/checkCLI.ts b/src/extensions/functions/checkCLI.ts
index 904da91..7a1ecac 100644
--- a/src/extensions/functions/checkCLI.ts
+++ b/src/extensions/functions/checkCLI.ts
@@ -20,9 +20,7 @@ module.exports = async (rule: SolidarityRule, context: SolidarityRunContext): Pr
 
     // I can't get no satisfaction
     if (!semver.satisfies(binarySemantic, rule.semver)) {
-      return `This system has an improper version for ${rule.binary}:
-        Rule='${rule.semver}'
-        Actual='${binaryVersion}'`
+      return `${rule.binary}: you have '${binaryVersion}', but the project requires '${rule.semver}'`
     }
   }
 }
diff --git a/src/extensions/functions/checkRequirement.ts b/src/extensions/functions/checkRequirement.ts
index 9de2dc5..7c70878 100644
--- a/src/extensions/functions/checkRequirement.ts
+++ b/src/extensions/functions/checkRequirement.ts
@@ -1,4 +1,4 @@
-import { SolidarityRequirement, SolidarityRunContext } from '../../types'
+import { SolidarityRequirement, SolidarityRunContext, SolidarityOutputMode } from '../../types'
 const checkCLI = require('./checkCLI')
 const checkENV = require('./checkENV')
 const checkDir = require('./checkDir')
@@ -13,13 +13,35 @@ module.exports = async (requirement: SolidarityRequirement, context: SolidarityR
   const rules = pipe(tail, flatten)(requirement)
 
   let ruleString = ''
-  const spinner = print.spin(`Verifying ${requirementName}`)
+  // Hide spinner if silent outputmode is set
+  const spinner = context.outputMode != SolidarityOutputMode.SILENT ? print.spin(`Verifying ${requirementName}`) : null
 
   const addFailure = (commonMessage, customMessage, ruleString) => {
-    spinner.fail(ruleString)
+    printResult(false, customMessage || commonMessage)
     return customMessage || commonMessage
   }
 
+  const printResult = (checkSuccessful, resultMessage) => {
+    switch(context.outputMode) {
+      case SolidarityOutputMode.VERBOSE:
+        // Print everything
+        checkSuccessful ? spinner.succeed(resultMessage) : spinner.fail(resultMessage)        
+        break;
+
+      case SolidarityOutputMode.SILENT: 
+        // Print nothing
+        break;
+
+      case SolidarityOutputMode.MODERATE:
+      default:
+        // Print only errors
+        if (!checkSuccessful) {
+          spinner.fail(resultMessage)
+        }
+        break;
+    }    
+  }
+
   // check each rule for requirement
   const ruleChecks = await map(async (rule) => {
     // Make sure this rule is active
@@ -33,7 +55,7 @@ module.exports = async (requirement: SolidarityRequirement, context: SolidarityR
         if (cliResult) {
           return addFailure(cliResult, rule.error, ruleString)
         } else {
-          spinner.succeed(ruleString)
+          printResult(true, ruleString)
           return []
         }
       // Handle ENV rule check
@@ -41,7 +63,7 @@ module.exports = async (requirement: SolidarityRequirement, context: SolidarityR
         const envResult = await checkENV(rule, context)
         ruleString = `${requirementName} - ${rule.variable} env`
         if (envResult) {
-          spinner.succeed(ruleString)
+          printResult(true, ruleString)
           return []
         } else {
           return addFailure(`'$${rule.variable}' environment variable not found`, rule.error, ruleString)
@@ -51,7 +73,7 @@ module.exports = async (requirement: SolidarityRequirement, context: SolidarityR
         const dirResult = checkDir(rule, context)
         ruleString = `${requirementName} - ${rule.location} directory`
         if (dirResult) {
-          spinner.succeed(ruleString)
+          printResult(true, ruleString)
           return []
         } else {
           return addFailure(`'$${rule.location}' directory not found`, rule.error, ruleString)
@@ -61,7 +83,7 @@ module.exports = async (requirement: SolidarityRequirement, context: SolidarityR
         const fileResult = checkFile(rule, context)
         ruleString = `${requirementName} - ${rule.location} file`
         if (fileResult) {
-          spinner.succeed(ruleString)
+          printResult(true, ruleString)
           return []
         } else {
           return addFailure(`'$${rule.location}' file not found`, rule.error, ruleString)
@@ -74,7 +96,9 @@ module.exports = async (requirement: SolidarityRequirement, context: SolidarityR
   // Run all the rule checks for a requirement
   return Promise.all(ruleChecks)
     .then(results => {
-      spinner.stop()
+      if (spinner != null) {
+        spinner.stop()
+      }
       return results
     })
     .catch(err => print.error(err))
diff --git a/src/types.ts b/src/types.ts
index b7ace62..9331ad9 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -27,6 +27,7 @@ export interface SolidarityRunContext extends GluegunRunContext {
   _pluginsList: Array<SolidarityPlugin & {templateDirectory: string}>
   addPlugin: (config: SolidarityPlugin) => void
   printSeparator: () => void
+  outputMode : SolidarityOutputMode
 }
 
 export type SnapshotType = (context: SolidarityRunContext) => Promise<void>
@@ -48,4 +49,11 @@ export type SolidarityRequirement = SolidarityRule[]
 
 export interface SolidaritySettings {
   requirements: object
+  config : object
 }
+
+export enum SolidarityOutputMode {
+  MODERATE,
+  VERBOSE,
+  SILENT
+}
\ No newline at end of file


Test patch:
diff --git a/__tests__/command_helpers/checkCLI.ts b/__tests__/command_helpers/checkCLI.ts
index bf375f8..d8ddee8 100644
--- a/__tests__/command_helpers/checkCLI.ts
+++ b/__tests__/command_helpers/checkCLI.ts
@@ -27,9 +27,7 @@ test('fine on existing binary', async () => {
 test('returns message on improper version', async () => {
   solidarityExtension(context)
   context.solidarity.getVersion = () => '1'
-  const message = `This system has an improper version for ${outOfDateCLI.binary}:
-        Rule='${outOfDateCLI.semver}'
-        Actual='1'`
+  const message = `${outOfDateCLI.binary}: you have '1', but the project requires '${outOfDateCLI.semver}'`
 
   expect(await checkCLI(outOfDateCLI, context)).toBe(message)
 })