Skip to content

Commit

Permalink
SNOW-950429: Set ESLint no-console rule to error (#676)
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-pbulawa authored Oct 31, 2023
1 parent cf17fa0 commit f49e3ef
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 37 deletions.
11 changes: 9 additions & 2 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,14 @@ module.exports = {
'node': true
},
'extends': 'eslint:recommended',
'overrides': [],
'overrides': [
{
'files': ['samples/*.js'],
'rules': {
'no-console': ['warn'],
}
}
],
'parserOptions': {
'ecmaVersion': 'latest'
},
Expand All @@ -25,7 +32,7 @@ module.exports = {
'keyword-spacing': ['warn'],
'linebreak-style': ['warn', 'unix'],
'no-async-promise-executor': ['warn'],
'no-console': ['warn', { 'allow': ['warn', 'error'] }],
'no-console': ['error', { 'allow': ['warn', 'error'] }],
'no-empty': ['warn'],
'no-ex-assign': ['warn'],
'no-extra-semi': ['warn'],
Expand Down
2 changes: 1 addition & 1 deletion lib/agent/https_proxy_ocsp_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ function HttpsProxyOcspAgent(opts)

if (!HttpsAgent.secureProxy)
{
console.warn("Warning: connecting to an authenticated proxy server through HTTP. To use HTTPS, set 'proxyProtocol' to 'HTTPS'")
Logger.getInstance().warn("Warning: connecting to an authenticated proxy server through HTTP. To use HTTPS, set 'proxyProtocol' to 'HTTPS'")
}
}

Expand Down
3 changes: 2 additions & 1 deletion lib/connection/connection_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const DEFAULT_PARAMS =
'disableQueryContextCache',
'loginTimeout',
];
const Logger = require('../logger');

function consolidateHostAndAccount(options)
{
Expand Down Expand Up @@ -499,7 +500,7 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo)
if (!DEFAULT_PARAMS.includes(key))
{
const result = levenshtein.closest(key, DEFAULT_PARAMS);
console.error(`"${key}" is an unknown connection parameter. Did you mean "${result}"?`);
Logger.getInstance().error(`'${key}' is an unknown connection parameter. Did you mean '${result}'?`);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ function Core(options)
{
if (err)
{
console.error('Unable to connect: ' + err.message);
Logger.getInstance().error('Unable to connect: ' + err.message);
reject(new Error(err.message));
}
else
Expand All @@ -299,7 +299,7 @@ function Core(options)
{
if (err)
{
console.error('Unable to disconnect: ' + err.message);
Logger.getInstance().error('Unable to disconnect: ' + err.message);
}
resolve();
});
Expand Down
31 changes: 13 additions & 18 deletions test/integration/testConnection.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ const connOption = require("./connectionOptions");
const testUtil = require("./testUtil");
const Util = require("./../../lib/util");
const Core = require("./../../lib/core");
const stderr = require("test-console").stderr;
const { stdout } = require('test-console');
const { assertLogMessage } = require('./testUtil');

describe("Connection test", function () {
it("return tokens in qaMode", function () {
Expand Down Expand Up @@ -111,7 +112,7 @@ describe("Connection test", function () {

describe("Connection test - validate default parameters", function () {
it('Valid "warehouse" parameter', function () {
const output = stderr.inspectSync(() => {
const output = stdout.inspectSync(() => {
snowflake.createConnection({
account: connOption.valid.account,
username: connOption.valid.username,
Expand All @@ -124,7 +125,7 @@ describe("Connection test - validate default parameters", function () {
});

it('Invalid "warehouse" parameter', function () {
const output = stderr.inspectSync(() => {
const output = stdout.inspectSync(() => {
snowflake.createConnection({
account: connOption.valid.account,
username: connOption.valid.username,
Expand All @@ -133,13 +134,11 @@ describe("Connection test - validate default parameters", function () {
validateDefaultParameters: true,
});
});
assert.deepEqual(output, [
'"waerhouse" is an unknown connection parameter. Did you mean "warehouse"?\n',
]);
assertLogMessage('ERROR', '\'waerhouse\' is an unknown connection parameter. Did you mean \'warehouse\'?', output[0]);
});

it('Valid "database" parameter', function () {
const output = stderr.inspectSync(() => {
const output = stdout.inspectSync(() => {
snowflake.createConnection({
account: connOption.valid.account,
username: connOption.valid.username,
Expand All @@ -152,7 +151,7 @@ describe("Connection test - validate default parameters", function () {
});

it('Invalid "db" parameter', function () {
const output = stderr.inspectSync(() => {
const output = stdout.inspectSync(() => {
snowflake.createConnection({
account: connOption.valid.account,
username: connOption.valid.username,
Expand All @@ -161,11 +160,11 @@ describe("Connection test - validate default parameters", function () {
validateDefaultParameters: true,
});
});
assert.deepEqual(output, ['"db" is an unknown connection parameter. Did you mean "host"?\n']);
assertLogMessage('ERROR', '\'db\' is an unknown connection parameter. Did you mean \'host\'?', output[0]);
});

it('Invalid "database" parameter', function () {
const output = stderr.inspectSync(() => {
const output = stdout.inspectSync(() => {
snowflake.createConnection({
account: connOption.valid.account,
username: connOption.valid.username,
Expand All @@ -174,13 +173,11 @@ describe("Connection test - validate default parameters", function () {
validateDefaultParameters: true,
});
});
assert.deepEqual(output, [
'"datbse" is an unknown connection parameter. Did you mean "database"?\n',
]);
assertLogMessage('ERROR', '\'datbse\' is an unknown connection parameter. Did you mean \'database\'?', output[0]);
});

it('Valid "schema" parameter', function () {
const output = stderr.inspectSync(() => {
const output = stdout.inspectSync(() => {
snowflake.createConnection({
account: connOption.valid.account,
username: connOption.valid.username,
Expand All @@ -193,7 +190,7 @@ describe("Connection test - validate default parameters", function () {
});

it('Invalid "schema" parameter', function () {
const output = stderr.inspectSync(() => {
const output = stdout.inspectSync(() => {
snowflake.createConnection({
account: connOption.valid.account,
username: connOption.valid.username,
Expand All @@ -202,9 +199,7 @@ describe("Connection test - validate default parameters", function () {
validateDefaultParameters: true,
});
});
assert.deepEqual(output, [
'"shcema" is an unknown connection parameter. Did you mean "schema"?\n',
]);
assertLogMessage('ERROR', '\'shcema\' is an unknown connection parameter. Did you mean \'schema\'?', output[0]);
});
});

Expand Down
3 changes: 2 additions & 1 deletion test/integration/testManualConnection.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const async = require("async");
const assert = require("assert");
const connOption = require("./connectionOptions");
const testUtil = require("./testUtil");
const Logger = require('../../lib/logger');

if (process.env.RUN_MANUAL_TESTS_ONLY == "true") {
describe.only("Run manual tests", function () {
Expand Down Expand Up @@ -321,7 +322,7 @@ if (process.env.RUN_MANUAL_TESTS_ONLY == "true") {
let time = await executeSingleQuery();
sumWithoutKeepAlive += time;
}
console.log(`Sum of time without keep alive: ${sumWithoutKeepAlive}. Sum of time with keep alive:: ${sumWithKeepAlive}`);
Logger.getInstance().info(`Sum of time without keep alive: ${sumWithoutKeepAlive}. Sum of time with keep alive:: ${sumWithKeepAlive}`);
assert.ok(sumWithoutKeepAlive * 0.66 > sumWithKeepAlive, 'With keep alive the queries should work faster');
});
});
Expand Down
14 changes: 7 additions & 7 deletions test/integration/testMultiStatement.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ var async = require('async');
var assert = require('assert');
var testUtil = require('./testUtil');
var Util = require('./../../lib/util');

const Logger = require('../../lib/logger');

describe('Test multi statement', function ()
{
Expand Down Expand Up @@ -41,9 +41,9 @@ describe('Test multi statement', function ()
connection.execute({
sqlText: 'select current_version()',
complete: function (err, stmt, rows) {
console.log('=== driver version = ' + Util.driverVersion);
console.log('=== server version =');
console.log(rows);
Logger.getInstance().info('=== driver version = ' + Util.driverVersion);
Logger.getInstance().info('=== server version =');
Logger.getInstance().info(rows);
callback();
}
});
Expand All @@ -61,14 +61,14 @@ describe('Test multi statement', function ()
testUtil.checkError(err);
});
stream.on('data', function (row) {
console.log(row);
Logger.getInstance().info(row);
count += Object.values(row).length;
if (stmt.hasNext()) {
console.log('==== hasNext');
Logger.getInstance().info('==== hasNext');
stmt.NextResult();
}
else {
console.log('==== close connection');
Logger.getInstance().info('==== close connection');
assert.strictEqual(6, count);
done();
}
Expand Down
2 changes: 1 addition & 1 deletion test/integration/testOcsp.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ describe('OCSP validation', function ()
{
if (!err.hasOwnProperty('code'))
{
console.log(err);
Logger.getInstance().error(err);
}
assert.equal(err['code'], '390100');
}
Expand Down
19 changes: 15 additions & 4 deletions test/integration/testUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const connOptions = require('./connectionOptions');
const assert = require('assert');
const fs = require('fs');
const crypto = require('crypto');
const Logger = require('../../lib/logger');

module.exports.createConnection = function (validConnectionOptionsOverride = {}) {
return snowflake.createConnection({
Expand Down Expand Up @@ -97,7 +98,7 @@ module.exports.dropTablesIgnoringErrorsAsync = async (connection, tableNames) =>
try {
await executeCmdAsync(connection, `DROP TABLE IF EXISTS ${tableName}`);
} catch (e) {
console.warn(`Cannot drop table ${tableName}: ${JSON.stringify(e)}`);
Logger.getInstance().warn(`Cannot drop table ${tableName}: ${JSON.stringify(e)}`);
}
}
};
Expand All @@ -113,7 +114,7 @@ module.exports.dropDBsIgnoringErrorsAsync = async (connection, dbNames) => {
try {
await executeCmdAsync(connection, `DROP DATABASE IF EXISTS ${dbName}`);
} catch (e) {
console.warn(`Cannot drop database ${dbName}: ${JSON.stringify(e)}`);
Logger.getInstance().warn(`Cannot drop database ${dbName}: ${JSON.stringify(e)}`);
}
}
};
Expand Down Expand Up @@ -226,7 +227,7 @@ module.exports.deleteFileSyncIgnoringErrors = function (file) {
fs.closeSync(file.fd);
fs.unlinkSync(file.name);
} catch (e) {
console.warn(`Cannot remove file ${file.name}: ${JSON.stringify(e)}`);
Logger.getInstance().warn(`Cannot remove file ${file.name}: ${JSON.stringify(e)}`);
}
}
};
Expand All @@ -242,7 +243,7 @@ module.exports.deleteFolderSyncIgnoringErrors = function (directory) {
fs.rmdirSync(directory, { recursive: true });
}
} catch (e) {
console.warn(`Cannot delete folder ${directory}: ${JSON.stringify(e)}`);
Logger.getInstance().warn(`Cannot delete folder ${directory}: ${JSON.stringify(e)}`);
}
};

Expand All @@ -256,3 +257,13 @@ module.exports.randomizeName = function (name) {
const randomString = crypto.randomBytes(4).toString('hex');
return name.concat(randomString);
};

/**
* @param expectedLevel string
* @param expectedMessage string
* @param actualMessage string
*/
module.exports.assertLogMessage = function (expectedLevel, expectedMessage, actualMessage) {
const regexPattern = `^{"level":"${expectedLevel}","message":"\\[.*\\]: ${expectedMessage}`;
return assert.match(actualMessage, new RegExp(regexPattern));
};

0 comments on commit f49e3ef

Please sign in to comment.