fix tests not looking at the errors from the console

DetachHead · Dec 14, 2024 · 2431b79 · 2431b79
1 parent 8d8e76c
commit 2431b79
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 20 deletions.
diff --git a/packages/pyright-internal/src/common/console.ts b/packages/pyright-internal/src/common/console.ts
@@ -84,7 +84,7 @@ export class NullConsole implements ConsoleInterface {
 
 export class StandardConsole implements ConsoleInterface {
     /** useful for determining whether to exit with a non-zero exit code */
-    errors = new Array<string>();
+    errorWasLogged = false;
     constructor(private _maxLevel: LogLevel = LogLevel.Log) {}
 
     get level(): LogLevel {
@@ -110,7 +110,9 @@ export class StandardConsole implements ConsoleInterface {
     }
 
     error(message: string) {
-        this.errors.push(message);
+        if (this.errorWasLogged) {
+            this.errorWasLogged = true;
+        }
         if (getLevelNumber(this._maxLevel) >= getLevelNumber(LogLevel.Error)) {
             console.error(message);
         }

diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts
@@ -548,8 +548,8 @@ const checkForErrors = (
     configParseErrorOccurred: boolean,
     console: ConsoleInterface
 ): boolean => {
-    if (console instanceof StandardConsole && console.errors.length > 0) {
-        console.errors = [];
+    if (console instanceof StandardConsole && console.errorWasLogged) {
+        console.errorWasLogged = false;
         exitStatus.resolve(ExitStatus.ConfigFileParseError);
         return true;
     }

diff --git a/packages/pyright-internal/src/tests/config.test.ts b/packages/pyright-internal/src/tests/config.test.ts
@@ -27,9 +27,18 @@ import { AnalysisResults } from '../analyzer/analysis';
 import { existsSync } from 'fs';
 import { NoAccessHost } from '../common/host';
 
+class ErrorTrackingNullConsole extends NullConsole {
+    errors = new Array<string>();
+
+    override error(message: string) {
+        this.errors.push(message);
+        super.error(message);
+    }
+}
+
 function createAnalyzer(console?: ConsoleInterface) {
     const tempFile = new RealTempFile();
-    const cons = console ?? new NullConsole();
+    const cons = console ?? new ErrorTrackingNullConsole();
     const fs = createFromRealFileSystem(tempFile, cons);
     const serviceProvider = createServiceProvider(fs, cons, tempFile);
     return new AnalyzerService('<default>', serviceProvider, { console: cons });
@@ -227,12 +236,12 @@ describe('invalid config', () => {
         const json = { asdf: 1 };
 
         const fs = new TestFileSystem(/* ignoreCase */ false);
-        const nullConsole = new NullConsole();
+        const console = new ErrorTrackingNullConsole();
 
-        const sp = createServiceProvider(fs, nullConsole);
-        const errors = configOptions.initializeFromJson(json, cwd, sp, new NoAccessHost());
+        const sp = createServiceProvider(fs, console);
+        configOptions.initializeFromJson(json, cwd, sp, new NoAccessHost());
 
-        assert.deepStrictEqual(errors, ['unknown config option: asdf']);
+        assert.deepStrictEqual(console.errors, ['unknown config option: asdf']);
     });
     test('unknown value for top-level option', () => {
         const cwd = UriEx.file(normalizePath(process.cwd()));
@@ -242,22 +251,20 @@ describe('invalid config', () => {
         const json = { typeCheckingMode: 'asdf' };
 
         const fs = new TestFileSystem(/* ignoreCase */ false);
-        const nullConsole = new NullConsole();
+        const console = new ErrorTrackingNullConsole();
 
-        const sp = createServiceProvider(fs, nullConsole);
-        const errors = configOptions.initializeFromJson(json, cwd, sp, new NoAccessHost());
+        const sp = createServiceProvider(fs, console);
+        configOptions.initializeFromJson(json, cwd, sp, new NoAccessHost());
 
-        assert.deepStrictEqual(errors, [
+        assert.deepStrictEqual(console.errors, [
             'invalid "typeCheckingMode" value: "asdf". expected: "off", "basic", "standard", "strict", "recommended", or "all"',
         ]);
     });
     test('unknown value in execution environments', () => {
-        const { analysisResult } = setupPyprojectToml(
+        const { consoleErrors } = setupPyprojectToml(
             'src/tests/samples/project_with_invalid_option_in_execution_environments'
         );
-        assert.deepStrictEqual(analysisResult?.configParseErrorOccurred, [
-            `unknown config option in execution environment "foo": asdf`,
-        ]);
+        assert.deepStrictEqual(consoleErrors, [`unknown config option in execution environment "foo": asdf`]);
     });
 });
 
@@ -348,18 +355,24 @@ test('AutoSearchPathsOnAndExtraPaths', () => {
 
 const setupPyprojectToml = (
     projectPath: string
-): { configOptions: ConfigOptions; analysisResult: AnalysisResults | undefined } => {
+): {
+    configOptions: ConfigOptions;
+    analysisResult: AnalysisResults | undefined;
+    consoleErrors: string[];
+} => {
     const cwd = normalizePath(combinePaths(process.cwd(), projectPath));
     assert(existsSync(cwd));
-    const service = createAnalyzer();
+    const console = new ErrorTrackingNullConsole();
+    const service = createAnalyzer(console);
     let analysisResult = undefined as AnalysisResults | undefined;
     service.setCompletionCallback((result) => (analysisResult = result));
     const commandLineOptions = new CommandLineOptions(cwd, /* fromLanguageServer */ true);
 
     service.setOptions(commandLineOptions);
 
     return {
-        configOptions: service.test_getConfigOptions(commandLineOptions),
+        configOptions: service.getConfigOptions(),
+        consoleErrors: console.errors,
         analysisResult,
     };
 };