Skip to content

Commit fe1a91a

Browse files
Kartik Rajericsnowcurrently
Kartik Raj
authored and
ericsnowcurrently
committed
Monkeypatch console.* calls on CI if we are asked to (#11897)
* Move console.* monkeypatching to src/test/testLogging.ts * Use monkeypatching in all tests launched using testBootstrap.ts * Use monkeypatching in single workspace and multiroot tests * News entry * Don't do monkeypatching for smoketests * Modify gulp task to not delete 'out/client/logging' directory * Undo smoke test ccheck * Import only from ./out/client/logging in test logger * Added comment * More corrections * Add comments * Correct smoke tests * Oops
1 parent 9bedf2d commit fe1a91a

12 files changed

Lines changed: 125 additions & 70 deletions

File tree

‎build/ci/templates/test_phases.yml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,12 +518,15 @@ steps:
518518
npm install -g vsce
519519
npm run clean
520520
npx tsc -p ./
521+
mkdir -p ./tmp/client/logging
522+
cp -r ./out/client/logging ./tmp/client
521523
npx gulp clean:cleanExceptTests
522-
mkdir -p ./tmp
523524
cp -r ./out/test ./tmp/test
524525
npm run updateBuildNumber -- --buildNumber $BUILD_BUILDID
525526
npm run package
526527
npx gulp clean:cleanExceptTests
528+
mkdir -p ./out/client/logging
529+
cp -r ./tmp/client/logging ./out/client
527530
cp -r ./tmp/test ./out/test
528531
node --no-force-async-hooks-checks ./out/test/smokeTest.js
529532
displayName: 'Run Smoke Tests'

‎news/3 Code Health/11896.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Monkeypatch `console.*` calls to the logger only in CI.

‎src/client/logging/_global.ts‎

Lines changed: 2 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@
33
'use strict';
44

55
import * as winston from 'winston';
6-
import { isCI } from '../common/constants';
76
import { IOutputChannel } from '../common/types';
87
import { CallInfo } from '../common/utils/decorators';
98
import { getFormatter } from './formatters';
109
import { LogLevel, resolveLevelName } from './levels';
11-
import { configureLogger, createLogger, ILogger, LoggerConfig, logToAll } from './logger';
10+
import { configureLogger, createLogger, getPreDefinedConfiguration, logToAll } from './logger';
1211
import { createTracingDecorator, LogInfo, TraceOptions, tracing as _tracing } from './trace';
1312
import { getPythonOutputChannelTransport } from './transports';
1413
import { Arguments } from './util';
@@ -40,35 +39,7 @@ initialize();
4039
* this is setup on CI servers.
4140
*/
4241
function initialize() {
43-
const config: LoggerConfig = {};
44-
let nonConsole = false;
45-
46-
// Do not log to console if running tests and we're not
47-
// asked to do so.
48-
if (process.env.VSC_PYTHON_FORCE_LOGGING) {
49-
config.console = {};
50-
// In CI there's no need for the label.
51-
if (!isCI) {
52-
config.console.label = 'Python Extension:';
53-
}
54-
}
55-
if (process.env.VSC_PYTHON_LOG_FILE) {
56-
config.file = {
57-
logfile: process.env.VSC_PYTHON_LOG_FILE
58-
};
59-
nonConsole = true;
60-
}
61-
configureLogger(globalLogger, config);
62-
63-
if (isCI && nonConsole) {
64-
delete config.console;
65-
// Send console.*() to the non-console loggers.
66-
monkeypatchConsole(
67-
// This is a separate logger that matches our config but
68-
// does not do any console logging.
69-
createLogger(config)
70-
);
71-
}
42+
configureLogger(globalLogger, getPreDefinedConfiguration());
7243
}
7344

7445
// Set the logging level the extension logs at.
@@ -144,37 +115,3 @@ export namespace traceDecorators {
144115
return createTracingDecorator([globalLogger], { message, opts, level });
145116
}
146117
}
147-
148-
/**
149-
* What we're doing here is monkey patching the console.log so we can
150-
* send everything sent to console window into our logs. This is only
151-
* required when we're directly writing to `console.log` or not using
152-
* our `winston logger`. This is something we'd generally turn on, only
153-
* on CI so we can see everything logged to the console window
154-
* (via the logs).
155-
*/
156-
function monkeypatchConsole(logger: ILogger) {
157-
// The logging "streams" (methods) of the node console.
158-
const streams = ['log', 'error', 'warn', 'info', 'debug', 'trace'];
159-
const levels: { [key: string]: LogLevel } = {
160-
error: LogLevel.Error,
161-
warn: LogLevel.Warn
162-
};
163-
// tslint:disable-next-line:no-any
164-
const consoleAny: any = console;
165-
for (const stream of streams) {
166-
// Using symbols guarantee the properties will be unique & prevents
167-
// clashing with names other code/library may create or have created.
168-
// We could use a closure but it's a bit trickier.
169-
const sym = Symbol.for(stream);
170-
consoleAny[sym] = consoleAny[stream];
171-
// tslint:disable-next-line: no-function-expression
172-
consoleAny[stream] = function () {
173-
const args = Array.prototype.slice.call(arguments);
174-
const fn = consoleAny[sym];
175-
fn(...args);
176-
const level = levels[stream] || LogLevel.Info;
177-
logToAll([logger], level, args);
178-
};
179-
}
180-
}

‎src/client/logging/levels.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// Licensed under the MIT License.
33
'use strict';
44

5+
// IMPORTANT: This file should only be importing from the '../client/logging' directory, as we
6+
// delete everything in '../client' except for '../client/logging' before running smoke tests.
7+
58
import * as winston from 'winston';
69

710
//======================

‎src/client/logging/logger.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// Licensed under the MIT License.
33
'use strict';
44

5+
// IMPORTANT: This file should only be importing from the '../client/logging' directory, as we
6+
// delete everything in '../client' except for '../client/logging' before running smoke tests.
7+
58
import * as util from 'util';
69
import * as winston from 'winston';
710
import * as Transport from 'winston-transport';
@@ -36,6 +39,34 @@ interface IConfigurableLogger {
3639
add(transport: Transport): void;
3740
}
3841

42+
// tslint:disable-next-line: no-suspicious-comment
43+
/**
44+
* TODO: We should actually have this method in `./_global.ts` as this is exported globally.
45+
* But for some reason, importing '../client/logging/_global' fails when launching the tests.
46+
* More details in the comment https://github.com/microsoft/vscode-python/pull/11897#discussion_r433954993
47+
* https://github.com/microsoft/vscode-python/issues/12137
48+
*/
49+
export function getPreDefinedConfiguration(): LoggerConfig {
50+
const config: LoggerConfig = {};
51+
52+
// Do not log to console if running tests and we're not
53+
// asked to do so.
54+
if (process.env.VSC_PYTHON_FORCE_LOGGING) {
55+
config.console = {};
56+
// In CI there's no need for the label.
57+
const isCI = process.env.TRAVIS === 'true' || process.env.TF_BUILD !== undefined;
58+
if (!isCI) {
59+
config.console.label = 'Python Extension:';
60+
}
61+
}
62+
if (process.env.VSC_PYTHON_LOG_FILE) {
63+
config.file = {
64+
logfile: process.env.VSC_PYTHON_LOG_FILE
65+
};
66+
}
67+
return config;
68+
}
69+
3970
// Set up a logger just the way we like it.
4071
export function configureLogger(logger: IConfigurableLogger, config: LoggerConfig) {
4172
if (config.level) {

‎src/client/logging/transports.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,21 @@
22
// Licensed under the MIT License.
33
'use strict';
44

5+
// IMPORTANT: This file should only be importing from the '../client/logging' directory, as we
6+
// delete everything in '../client' except for '../client/logging' before running smoke tests.
7+
58
import * as logform from 'logform';
69
import * as path from 'path';
10+
import { OutputChannel } from 'vscode';
711
import * as winston from 'winston';
812
import * as Transport from 'winston-transport';
9-
import { IOutputChannel } from '../common/types';
10-
import { EXTENSION_ROOT_DIR } from '../constants';
1113
import { LogLevel, resolveLevel } from './levels';
1214
import { Arguments } from './util';
1315

16+
const folderPath = path.dirname(__dirname);
17+
const folderName = path.basename(folderPath);
18+
const EXTENSION_ROOT_DIR =
19+
folderName === 'client' ? path.join(folderPath, '..', '..') : path.join(folderPath, '..', '..', '..', '..');
1420
const formattedMessage = Symbol.for('message');
1521

1622
export function isConsoleTransport(transport: unknown): boolean {
@@ -74,7 +80,7 @@ export function getConsoleTransport(formatter: logform.Format): Transport {
7480

7581
class PythonOutputChannelTransport extends Transport {
7682
// tslint:disable-next-line: no-any
77-
constructor(private readonly channel: IOutputChannel, options?: any) {
83+
constructor(private readonly channel: OutputChannel, options?: any) {
7884
super(options);
7985
}
8086
// tslint:disable-next-line: no-any
@@ -88,7 +94,7 @@ class PythonOutputChannelTransport extends Transport {
8894
}
8995

9096
// Create a Python output channel targeting transport that can be added to a winston logger.
91-
export function getPythonOutputChannelTransport(channel: IOutputChannel, formatter: logform.Format) {
97+
export function getPythonOutputChannelTransport(channel: OutputChannel, formatter: logform.Format) {
9298
return new PythonOutputChannelTransport(channel, {
9399
// We minimize customization.
94100
format: formatter

‎src/test/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ import {
2323
TEST_TIMEOUT
2424
} from './constants';
2525
import { initialize } from './initialize';
26+
import { initializeLogger } from './testLogger';
27+
28+
initializeLogger();
2629

2730
type SetupOptions = Mocha.MochaOptions & {
2831
testFilesSuffix: string;

‎src/test/multiRootTest.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@
33
import * as path from 'path';
44
import { runTests } from 'vscode-test';
55
import { EXTENSION_ROOT_DIR_FOR_TESTS } from './constants';
6+
import { initializeLogger } from './testLogger';
67

78
const workspacePath = path.join(__dirname, '..', '..', 'src', 'testMultiRootWkspc', 'multi.code-workspace');
89
process.env.IS_CI_SERVER_TEST_DEBUGGER = '';
910
process.env.VSC_PYTHON_CI_TEST = '1';
1011

12+
initializeLogger();
13+
1114
function start() {
1215
console.log('*'.repeat(100));
1316
console.log('Start Multiroot tests');

‎src/test/performanceTest.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ import * as request from 'request';
2525
import { LanguageServerType } from '../client/activation/types';
2626
import { EXTENSION_ROOT_DIR, PVSC_EXTENSION_ID } from '../client/common/constants';
2727
import { unzip } from './common';
28+
import { initializeLogger } from './testLogger';
29+
30+
initializeLogger();
2831

2932
const NamedRegexp = require('named-js-regexp');
3033
const del = require('del');

‎src/test/standardTest.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
import * as path from 'path';
44
import { runTests } from 'vscode-test';
55
import { EXTENSION_ROOT_DIR_FOR_TESTS } from './constants';
6+
import { initializeLogger } from './testLogger';
7+
8+
initializeLogger();
69

710
process.env.IS_CI_SERVER_TEST_DEBUGGER = '';
811
process.env.VSC_PYTHON_CI_TEST = '1';

0 commit comments

Comments
 (0)