From c8d9f88e97b5f75b510c15541d359a0b37839499 Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Mon, 1 Aug 2022 17:52:02 -0400 Subject: [PATCH 1/6] feat(logger): add GHA-aware notices/warnings --- .eslintrc | 2 +- __tests__/cmds/docs.test.js | 1 + __tests__/cmds/openapi.test.js | 1 + __tests__/cmds/validate.test.js | 1 + __tests__/lib/fetch.test.js | 2 ++ package-lock.json | 14 ++++++++++++++ package.json | 1 + src/cmds/docs/edit.js | 4 ++-- src/cmds/openapi.js | 12 +++++------- src/cmds/swagger.js | 5 ++--- src/lib/isGitHub.js | 4 +++- src/lib/logger.js | 23 +++++++++++++++++++++++ src/lib/prepareOas.js | 4 ++-- 13 files changed, 58 insertions(+), 16 deletions(-) diff --git a/.eslintrc b/.eslintrc index 6550a91ee..8b9d3ec82 100644 --- a/.eslintrc +++ b/.eslintrc @@ -21,6 +21,6 @@ * so we can write robust tests and take advantage of `bin/rdme`, * which we use for printing function outputs and returning correct exit codes. */ - "no-console": ["warn", { "allow": ["info", "warn"] }] + "no-console": ["warn"] } } diff --git a/__tests__/cmds/docs.test.js b/__tests__/cmds/docs.test.js index bf2b23d2f..fd2cb3640 100644 --- a/__tests__/cmds/docs.test.js +++ b/__tests__/cmds/docs.test.js @@ -1,3 +1,4 @@ +/* eslint-disable no-console */ const nock = require('nock'); const chalk = require('chalk'); const fs = require('fs'); diff --git a/__tests__/cmds/openapi.test.js b/__tests__/cmds/openapi.test.js index 0ace20b41..927d12951 100644 --- a/__tests__/cmds/openapi.test.js +++ b/__tests__/cmds/openapi.test.js @@ -1,3 +1,4 @@ +/* eslint-disable no-console */ const nock = require('nock'); const chalk = require('chalk'); const config = require('config'); diff --git a/__tests__/cmds/validate.test.js b/__tests__/cmds/validate.test.js index bd0f59355..9218ff9d3 100644 --- a/__tests__/cmds/validate.test.js +++ b/__tests__/cmds/validate.test.js @@ -1,3 +1,4 @@ +/* eslint-disable no-console */ const fs = require('fs'); const chalk = require('chalk'); const Command = require('../../src/cmds/validate'); diff --git a/__tests__/lib/fetch.test.js b/__tests__/lib/fetch.test.js index a92c69236..29d10ad2b 100644 --- a/__tests__/lib/fetch.test.js +++ b/__tests__/lib/fetch.test.js @@ -9,6 +9,7 @@ describe('#fetch()', () => { // List of all GitHub Actions env variables: // https://docs.github.com/en/actions/learn-github-actions/environment-variables#default-environment-variables beforeEach(() => { + process.env.GITHUB_ACTION = '__repo-owner_name-of-action-repo'; process.env.GITHUB_ACTIONS = 'true'; process.env.GITHUB_REPOSITORY = 'octocat/Hello-World'; process.env.GITHUB_RUN_ATTEMPT = '3'; @@ -18,6 +19,7 @@ describe('#fetch()', () => { }); afterEach(() => { + delete process.env.GITHUB_ACTION; delete process.env.GITHUB_ACTIONS; delete process.env.GITHUB_REPOSITORY; delete process.env.GITHUB_RUN_ATTEMPT; diff --git a/package-lock.json b/package-lock.json index 64fa574e2..cb3937c7d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,6 +10,7 @@ "license": "MIT", "dependencies": { "@actions/core": "^1.6.0", + "@npmcli/ci-detect": "^2.0.0", "chalk": "^4.1.2", "cli-table": "^0.3.1", "command-line-args": "^5.2.0", @@ -1266,6 +1267,14 @@ "node": ">= 8" } }, + "node_modules/@npmcli/ci-detect": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@npmcli/ci-detect/-/ci-detect-2.0.0.tgz", + "integrity": "sha512-8yQtQ9ArHh/TzdUDKQwEvwCgpDuhSWTDAbiKMl3854PcT+Dk4UmWaiawuFTLy9n5twzXOBXVflWe+90/ffXQrA==", + "engines": { + "node": "^12.13.0 || ^14.15.0 || >=16" + } + }, "node_modules/@pkgr/utils": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/@pkgr/utils/-/utils-2.3.0.tgz", @@ -13183,6 +13192,11 @@ "fastq": "^1.6.0" } }, + "@npmcli/ci-detect": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@npmcli/ci-detect/-/ci-detect-2.0.0.tgz", + "integrity": "sha512-8yQtQ9ArHh/TzdUDKQwEvwCgpDuhSWTDAbiKMl3854PcT+Dk4UmWaiawuFTLy9n5twzXOBXVflWe+90/ffXQrA==" + }, "@pkgr/utils": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/@pkgr/utils/-/utils-2.3.0.tgz", diff --git a/package.json b/package.json index 385af1348..a1926f699 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,7 @@ }, "dependencies": { "@actions/core": "^1.6.0", + "@npmcli/ci-detect": "^2.0.0", "chalk": "^4.1.2", "cli-table": "^0.3.1", "command-line-args": "^5.2.0", diff --git a/src/cmds/docs/edit.js b/src/cmds/docs/edit.js index 58578457e..2fccae025 100644 --- a/src/cmds/docs/edit.js +++ b/src/cmds/docs/edit.js @@ -6,7 +6,7 @@ const APIError = require('../../lib/apiError'); const { getProjectVersion } = require('../../lib/versionSelect'); const fetch = require('../../lib/fetch'); const { cleanHeaders, handleRes } = require('../../lib/fetch'); -const { debug } = require('../../lib/logger'); +const { debug, info } = require('../../lib/logger'); const writeFile = promisify(fs.writeFile); const readFile = promisify(fs.readFile); @@ -101,7 +101,7 @@ module.exports = class EditDocsCommand { if (res.error) { return reject(new APIError(res)); } - console.info('Doc successfully updated. Cleaning up local file.'); + info('Doc successfully updated. Cleaning up local file.'); await unlink(filename); debug('file unlinked'); // Normally we should resolve with a value that is logged to the console, diff --git a/src/cmds/openapi.js b/src/cmds/openapi.js index aff820ca2..913b6a955 100644 --- a/src/cmds/openapi.js +++ b/src/cmds/openapi.js @@ -2,7 +2,7 @@ const APIError = require('../lib/apiError'); const chalk = require('chalk'); const { cleanHeaders } = require('../lib/fetch'); const config = require('config'); -const { debug, oraOptions } = require('../lib/logger'); +const { debug, warn, oraOptions } = require('../lib/logger'); const fetch = require('../lib/fetch'); const { handleRes } = require('../lib/fetch'); const { getProjectVersion } = require('../lib/versionSelect'); @@ -66,12 +66,10 @@ module.exports = class OpenAPICommand { } if (version && id) { - console.warn( - chalk.yellow( - `⚠️ Warning! We'll be using the version associated with the \`--${ - opts.token ? 'token' : 'id' - }\` option, so the \`--version\` option will be ignored.` - ) + warn( + `We'll be using the version associated with the \`--${ + opts.token ? 'token' : 'id' + }\` option, so the \`--version\` option will be ignored.` ); } diff --git a/src/cmds/swagger.js b/src/cmds/swagger.js index ade7ccea3..e25053698 100644 --- a/src/cmds/swagger.js +++ b/src/cmds/swagger.js @@ -1,6 +1,5 @@ -const chalk = require('chalk'); const OpenAPICommand = require('./openapi'); -const { debug } = require('../lib/logger'); +const { debug, warn } = require('../lib/logger'); module.exports = class SwaggerCommand extends OpenAPICommand { constructor() { @@ -16,7 +15,7 @@ module.exports = class SwaggerCommand extends OpenAPICommand { debug(`command: ${this.command}`); debug(`opts: ${JSON.stringify(opts)}`); - console.warn(chalk.yellow('⚠️ Warning! `rdme swagger` has been deprecated. Please use `rdme openapi` instead.')); + warn('`rdme swagger` has been deprecated. Please use `rdme openapi` instead.'); return super.run(opts); } }; diff --git a/src/lib/isGitHub.js b/src/lib/isGitHub.js index a8c9fb102..c12e4435c 100644 --- a/src/lib/isGitHub.js +++ b/src/lib/isGitHub.js @@ -1,7 +1,9 @@ +const ciDetect = require('@npmcli/ci-detect'); + /** * Small env check to determine if we're in a GitHub Actions environment * @link https://docs.github.com/en/actions/learn-github-actions/environment-variables#default-environment-variables */ module.exports = function isGHA() { - return process.env.GITHUB_ACTIONS === 'true'; + return ciDetect() === 'github-actions'; }; diff --git a/src/lib/logger.js b/src/lib/logger.js index c3abc4cdd..35500da36 100644 --- a/src/lib/logger.js +++ b/src/lib/logger.js @@ -1,3 +1,4 @@ +const chalk = require('chalk'); const config = require('config'); const core = require('@actions/core'); const debugPackage = require('debug')(config.get('cli')); @@ -13,6 +14,28 @@ module.exports.debug = function debug(input) { return debugPackage(input); }; +/** + * Wrapper for warn statements. + * @param {String} input + */ +module.exports.warn = function warn(input) { + /* istanbul ignore next */ + if (isGHA()) return core.warning(input); + // eslint-disable-next-line no-console + return console.warn(chalk.yellow(`⚠️ Warning! ${input}`)); +}; + +/** + * Wrapper for info/notice statements. + * @param {String} input + */ +module.exports.info = function info(input) { + /* istanbul ignore next */ + if (isGHA()) return core.notice(input); + // eslint-disable-next-line no-console + return console.info(input); +}; + module.exports.oraOptions = function oraOptions() { // Disables spinner in tests so it doesn't pollute test output const opts = { isSilent: process.env.NODE_ENV === 'testing' }; diff --git a/src/lib/prepareOas.js b/src/lib/prepareOas.js index f03a5be67..339e35afd 100644 --- a/src/lib/prepareOas.js +++ b/src/lib/prepareOas.js @@ -3,7 +3,7 @@ const fs = require('fs'); const OASNormalize = require('oas-normalize'); const ora = require('ora'); -const { debug, oraOptions } = require('./logger'); +const { debug, info, oraOptions } = require('./logger'); /** * Normalizes, validates, and (optionally) bundles an OpenAPI definition. @@ -27,7 +27,7 @@ module.exports = async function prepare(path, command) { return; } - console.info( + info( chalk.yellow(`We found ${file} and are attempting to ${command === 'openapi' ? 'upload' : 'validate'} it.`) ); resolve(file); From 92ae5ac4bfd246f93a2c00f8750faaac1a697acb Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Mon, 1 Aug 2022 17:55:28 -0400 Subject: [PATCH 2/6] chore: temporarily test warnings --- .github/workflows/ci.yml | 5 ++ __tests__/__fixtures__/openapi.json | 133 ++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+) create mode 100644 __tests__/__fixtures__/openapi.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2b982b9a0..8c1b39578 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,6 +67,11 @@ jobs: with: rdme: validate oas-examples-repo/3.1/json/petstore.json + - name: Run `validate` command to simulate warning + uses: ./rdme-repo/ + with: + rdme: validate --workingDirectory=rdme-repo/__tests__/__fixtures__ + # Docs: https://rdme-test.readme.io - name: Run `openapi` command uses: ./rdme-repo/ diff --git a/__tests__/__fixtures__/openapi.json b/__tests__/__fixtures__/openapi.json new file mode 100644 index 000000000..74ebf94fa --- /dev/null +++ b/__tests__/__fixtures__/openapi.json @@ -0,0 +1,133 @@ +{ + "openapi": "3.0.0", + "info": { + "title": "Endpoints", + "version": "1.0.0" + }, + "servers": [ + { + "url": "https://readme.nyc/api" + } + ], + "paths": { + "/message": { + "get": { + "summary": "Get the current message", + "description": "This will return the current message", + "tags": [ + "Messages" + ], + "security": [ + { + "ApiKeyAuth": [] + } + ], + "responses": { + "200": { + "description": "Successful response" + } + } + }, + "post": { + "summary": "Update the sign message", + "description": "This will send a message to the Vestaboard to be displayed", + "requestBody": { + "description": "Optional description in *Markdown*", + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "text", + "color" + ], + "properties": { + "text": { + "type": "string", + "description": "What text should be displayed?" + }, + "color": { + "type": "string", + "enum": [ + "red", + "orange", + "yellow", + "green", + "blue", + "violet" + ] + } + } + } + } + } + }, + "tags": [ + "Messages" + ], + "security": [ + { + "ApiKeyAuth": [] + } + ], + "responses": { + "200": { + "description": "Successful response" + } + } + }, + "delete": { + "summary": "Reset sign message", + "description": "This will reset the Vestaboard's message", + "tags": [ + "Messages" + ], + "security": [ + { + "ApiKeyAuth": [] + } + ], + "responses": { + "200": { + "description": "Successful response" + } + } + } + }, + "/owlfact": { + "get": { + "summary": "Owl facts", + "description": "Get a random, possibly true, owl fact", + "tags": [ + "Fun Facts" + ], + "security": [ + { + "ApiKeyAuth": [] + } + ], + "responses": { + "200": { + "description": "Successful response" + } + } + } + } + }, + "tags": [ + { + "name": "Messages", + "description": "Interact with the Vestaboard" + } + ], + "components": { + "securitySchemes": { + "ApiKeyAuth": { + "type": "apiKey", + "in": "header", + "name": "X-API-Key" + } + } + } +} From 8cde81a9a4236d95ad3e0f3266c17374cdc450d4 Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Mon, 1 Aug 2022 17:58:54 -0400 Subject: [PATCH 3/6] chore: fix logging in test suite --- src/lib/logger.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/logger.js b/src/lib/logger.js index 35500da36..6b888f10a 100644 --- a/src/lib/logger.js +++ b/src/lib/logger.js @@ -20,7 +20,7 @@ module.exports.debug = function debug(input) { */ module.exports.warn = function warn(input) { /* istanbul ignore next */ - if (isGHA()) return core.warning(input); + if (isGHA() && process.env.NODE_ENV !== 'testing') return core.warning(input); // eslint-disable-next-line no-console return console.warn(chalk.yellow(`⚠️ Warning! ${input}`)); }; @@ -31,7 +31,7 @@ module.exports.warn = function warn(input) { */ module.exports.info = function info(input) { /* istanbul ignore next */ - if (isGHA()) return core.notice(input); + if (isGHA() && process.env.NODE_ENV !== 'testing') return core.notice(input); // eslint-disable-next-line no-console return console.info(input); }; From 6de97715ee64bc2a3f2c270ea8fc35283eff2169 Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Mon, 1 Aug 2022 18:00:41 -0400 Subject: [PATCH 4/6] chore: temporarily simulate another warning --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8c1b39578..329dad959 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -76,4 +76,4 @@ jobs: - name: Run `openapi` command uses: ./rdme-repo/ with: - rdme: openapi oas-examples-repo/3.1/json/petstore.json --key=${{ secrets.RDME_TEST_PROJECT_API_KEY }} --id=${{ secrets.RDME_TEST_PROJECT_API_SETTING }} + rdme: swagger oas-examples-repo/3.1/json/petstore.json --key=${{ secrets.RDME_TEST_PROJECT_API_KEY }} --id=${{ secrets.RDME_TEST_PROJECT_API_SETTING }} From 6368b1bb033a55c56004390d18841d55cbf2e795 Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Mon, 1 Aug 2022 18:04:12 -0400 Subject: [PATCH 5/6] revert: restore CI to original state --- .github/workflows/ci.yml | 7 +- __tests__/__fixtures__/openapi.json | 133 ---------------------------- 2 files changed, 1 insertion(+), 139 deletions(-) delete mode 100644 __tests__/__fixtures__/openapi.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 329dad959..2b982b9a0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,13 +67,8 @@ jobs: with: rdme: validate oas-examples-repo/3.1/json/petstore.json - - name: Run `validate` command to simulate warning - uses: ./rdme-repo/ - with: - rdme: validate --workingDirectory=rdme-repo/__tests__/__fixtures__ - # Docs: https://rdme-test.readme.io - name: Run `openapi` command uses: ./rdme-repo/ with: - rdme: swagger oas-examples-repo/3.1/json/petstore.json --key=${{ secrets.RDME_TEST_PROJECT_API_KEY }} --id=${{ secrets.RDME_TEST_PROJECT_API_SETTING }} + rdme: openapi oas-examples-repo/3.1/json/petstore.json --key=${{ secrets.RDME_TEST_PROJECT_API_KEY }} --id=${{ secrets.RDME_TEST_PROJECT_API_SETTING }} diff --git a/__tests__/__fixtures__/openapi.json b/__tests__/__fixtures__/openapi.json deleted file mode 100644 index 74ebf94fa..000000000 --- a/__tests__/__fixtures__/openapi.json +++ /dev/null @@ -1,133 +0,0 @@ -{ - "openapi": "3.0.0", - "info": { - "title": "Endpoints", - "version": "1.0.0" - }, - "servers": [ - { - "url": "https://readme.nyc/api" - } - ], - "paths": { - "/message": { - "get": { - "summary": "Get the current message", - "description": "This will return the current message", - "tags": [ - "Messages" - ], - "security": [ - { - "ApiKeyAuth": [] - } - ], - "responses": { - "200": { - "description": "Successful response" - } - } - }, - "post": { - "summary": "Update the sign message", - "description": "This will send a message to the Vestaboard to be displayed", - "requestBody": { - "description": "Optional description in *Markdown*", - "required": true, - "content": { - "application/json": { - "schema": { - "type": "object", - "required": [ - "text", - "color" - ], - "properties": { - "text": { - "type": "string", - "description": "What text should be displayed?" - }, - "color": { - "type": "string", - "enum": [ - "red", - "orange", - "yellow", - "green", - "blue", - "violet" - ] - } - } - } - } - } - }, - "tags": [ - "Messages" - ], - "security": [ - { - "ApiKeyAuth": [] - } - ], - "responses": { - "200": { - "description": "Successful response" - } - } - }, - "delete": { - "summary": "Reset sign message", - "description": "This will reset the Vestaboard's message", - "tags": [ - "Messages" - ], - "security": [ - { - "ApiKeyAuth": [] - } - ], - "responses": { - "200": { - "description": "Successful response" - } - } - } - }, - "/owlfact": { - "get": { - "summary": "Owl facts", - "description": "Get a random, possibly true, owl fact", - "tags": [ - "Fun Facts" - ], - "security": [ - { - "ApiKeyAuth": [] - } - ], - "responses": { - "200": { - "description": "Successful response" - } - } - } - } - }, - "tags": [ - { - "name": "Messages", - "description": "Interact with the Vestaboard" - } - ], - "components": { - "securitySchemes": { - "ApiKeyAuth": { - "type": "apiKey", - "in": "header", - "name": "X-API-Key" - } - } - } -} From d7e0a9fdaa281e246163ca8e9a1b837d1da83293 Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Mon, 1 Aug 2022 18:11:26 -0400 Subject: [PATCH 6/6] docs: add explanation to eslintrc --- .eslintrc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.eslintrc b/.eslintrc index 8b9d3ec82..7cb7f394d 100644 --- a/.eslintrc +++ b/.eslintrc @@ -20,6 +20,9 @@ * We should be returning Promise-wrapped values in our main command functions * so we can write robust tests and take advantage of `bin/rdme`, * which we use for printing function outputs and returning correct exit codes. + * + * Furthermore, we should also be using our custom loggers (see src/lib/logger.js) + * instead of using console.info() or console.warn() statements. */ "no-console": ["warn"] }