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(logger): add GHA-aware notices/warnings #557

Merged
merged 6 commits into from
Aug 1, 2022
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
5 changes: 4 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
* 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", { "allow": ["info", "warn"] }]
"no-console": ["warn"]
}
}
1 change: 1 addition & 0 deletions __tests__/cmds/docs.test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-console */
const nock = require('nock');
const chalk = require('chalk');
const fs = require('fs');
Expand Down
1 change: 1 addition & 0 deletions __tests__/cmds/openapi.test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-console */
const nock = require('nock');
const chalk = require('chalk');
const config = require('config');
Expand Down
1 change: 1 addition & 0 deletions __tests__/cmds/validate.test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-console */
const fs = require('fs');
const chalk = require('chalk');
const Command = require('../../src/cmds/validate');
Expand Down
2 changes: 2 additions & 0 deletions __tests__/lib/fetch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this to further simulate a GHA environment in our tests since this is the variable that ci-detect uses.

process.env.GITHUB_ACTIONS = 'true';
process.env.GITHUB_REPOSITORY = 'octocat/Hello-World';
process.env.GITHUB_RUN_ATTEMPT = '3';
Expand All @@ -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;
Expand Down
14 changes: 14 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions src/cmds/docs/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 5 additions & 7 deletions src/cmds/openapi.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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.`
);
}

Expand Down
5 changes: 2 additions & 3 deletions src/cmds/swagger.js
Original file line number Diff line number Diff line change
@@ -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() {
Expand All @@ -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);
}
};
4 changes: 3 additions & 1 deletion src/lib/isGitHub.js
Original file line number Diff line number Diff line change
@@ -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';
};
23 changes: 23 additions & 0 deletions src/lib/logger.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const chalk = require('chalk');
const config = require('config');
const core = require('@actions/core');
const debugPackage = require('debug')(config.get('cli'));
Expand All @@ -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() && process.env.NODE_ENV !== 'testing') 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() && process.env.NODE_ENV !== 'testing') 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' };
Expand Down
4 changes: 2 additions & 2 deletions src/lib/prepareOas.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand Down