From c13319dd04aee59c0052ee32521df5d39a1dfa10 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Tue, 9 Aug 2022 16:35:30 +0200 Subject: [PATCH 01/30] Use sarif parser for reopened results --- extensions/ql-vscode/src/query-results.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/query-results.ts b/extensions/ql-vscode/src/query-results.ts index 34dd4f733d2..02281b5885a 100644 --- a/extensions/ql-vscode/src/query-results.ts +++ b/extensions/ql-vscode/src/query-results.ts @@ -17,6 +17,7 @@ import { import { DatabaseInfo } from './pure/interface-types'; import { QueryStatus } from './query-status'; import { RemoteQueryHistoryItem } from './remote-queries/remote-query-history-item'; +import { sarifParser } from './sarif-parser'; /** * query-results.ts @@ -160,10 +161,12 @@ export async function interpretResultsSarif( sourceInfo?: cli.SourceInfo ): Promise { const { resultsPath, interpretedResultsPath } = resultsPaths; + let res; if (await fs.pathExists(interpretedResultsPath)) { - return { ...JSON.parse(await fs.readFile(interpretedResultsPath, 'utf8')), t: 'SarifInterpretationData' }; + res = await sarifParser(interpretedResultsPath); + } else { + res = await cli.interpretBqrsSarif(ensureMetadataIsComplete(metadata), resultsPath, interpretedResultsPath, sourceInfo); } - const res = await cli.interpretBqrsSarif(ensureMetadataIsComplete(metadata), resultsPath, interpretedResultsPath, sourceInfo); return { ...res, t: 'SarifInterpretationData' }; } From b3d7e7813a7b0128de93b2586f67f0b5ea002f77 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Mon, 15 Aug 2022 09:49:16 -0700 Subject: [PATCH 02/30] Bump unit test timeout to 3 min --- .../src/vscode-tests/no-workspace/query-results.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 89a23028e2f..1647ce9ed1b 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -150,7 +150,10 @@ describe('query-results', () => { }); }); - it('should interpretResultsSarif', async () => { + it('should interpretResultsSarif', async function() { + // up to 3 minutes per test + this.timeout(3 * 60 * 1000); + const spy = sandbox.mock(); spy.returns({ a: '1234' }); const mockServer = { From c37c5bf6446fb0585044fba75b9cb92b8a5c348c Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Mon, 15 Aug 2022 09:57:34 -0700 Subject: [PATCH 03/30] Bump unit test timeout to 5 min --- .../src/vscode-tests/no-workspace/query-results.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 1647ce9ed1b..a15c16e126e 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -151,8 +151,8 @@ describe('query-results', () => { }); it('should interpretResultsSarif', async function() { - // up to 3 minutes per test - this.timeout(3 * 60 * 1000); + // up to 5 minutes per test + this.timeout(5 * 60 * 1000); const spy = sandbox.mock(); spy.returns({ a: '1234' }); From c5a816d67430a2f6a551017513c5a3a198edd6e9 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Tue, 9 Aug 2022 16:35:30 +0200 Subject: [PATCH 04/30] Use sarif parser for reopened results --- extensions/ql-vscode/src/query-results.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/ql-vscode/src/query-results.ts b/extensions/ql-vscode/src/query-results.ts index 785d2652ae6..0977fa056fc 100644 --- a/extensions/ql-vscode/src/query-results.ts +++ b/extensions/ql-vscode/src/query-results.ts @@ -20,6 +20,7 @@ import { RemoteQueryHistoryItem } from './remote-queries/remote-query-history-it import { sarifParser } from './sarif-parser'; import { QueryEvaluationInfo, QueryWithResults } from './run-queries-shared'; import { formatLegacyMessage } from './legacy-query-server/run-queries'; +import { sarifParser } from './sarif-parser'; /** * query-results.ts From c1b3ee1061180a13f1c74ed21c2570907d4e8a05 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Tue, 11 Oct 2022 16:14:17 -0700 Subject: [PATCH 05/30] Remove extra import statement --- extensions/ql-vscode/src/query-results.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/extensions/ql-vscode/src/query-results.ts b/extensions/ql-vscode/src/query-results.ts index 0977fa056fc..35c0bdcab7b 100644 --- a/extensions/ql-vscode/src/query-results.ts +++ b/extensions/ql-vscode/src/query-results.ts @@ -17,7 +17,6 @@ import { import { DatabaseInfo } from './pure/interface-types'; import { QueryStatus } from './query-status'; import { RemoteQueryHistoryItem } from './remote-queries/remote-query-history-item'; -import { sarifParser } from './sarif-parser'; import { QueryEvaluationInfo, QueryWithResults } from './run-queries-shared'; import { formatLegacyMessage } from './legacy-query-server/run-queries'; import { sarifParser } from './sarif-parser'; From 2b6dd6bd91dfab2ee1f953c9733f4d8127f35730 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Tue, 11 Oct 2022 16:15:21 -0700 Subject: [PATCH 06/30] Exit parser when invalid SARIF is parsed --- extensions/ql-vscode/src/sarif-parser.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/extensions/ql-vscode/src/sarif-parser.ts b/extensions/ql-vscode/src/sarif-parser.ts index c08d93c5ef2..3712ee73b3d 100644 --- a/extensions/ql-vscode/src/sarif-parser.ts +++ b/extensions/ql-vscode/src/sarif-parser.ts @@ -1,33 +1,34 @@ import * as Sarif from 'sarif'; import * as fs from 'fs-extra'; -import { parser } from 'stream-json'; -import { pick } from 'stream-json/filters/Pick'; import Assembler = require('stream-json/Assembler'); -import { chain } from 'stream-chain'; import { getErrorMessage } from './pure/helpers-pure'; +import Pick = require('stream-json/filters/Pick'); const DUMMY_TOOL: Sarif.Tool = { driver: { name: '' } }; export async function sarifParser(interpretedResultsPath: string): Promise { try { // Parse the SARIF file into token streams, filtering out only the results array. - const p = parser(); - const pipeline = chain([ - fs.createReadStream(interpretedResultsPath), - p, - pick({ filter: 'runs.0.results' }) - ]); + const pipeline = fs.createReadStream(interpretedResultsPath).pipe(Pick.withParser({ filter: 'runs.0.results' })); // Creates JavaScript objects from the token stream const asm = Assembler.connectTo(pipeline); - // Returns a constructed Log object with the results or an empty array if no results were found. + // Returns a constructed Log object with the results of an empty array if no results were found. // If the parser fails for any reason, it will reject the promise. return await new Promise((resolve, reject) => { + let alreadyDone = false; pipeline.on('error', (error) => { reject(error); }); + // If the parser pipeline completes before the assembler, we've reached end of file and have not found any results. + pipeline.on('end', () => { + if (!alreadyDone) { + reject(new Error('Invalid SARIF file: expecting at least one run with result.')); + } + }); + asm.on('done', (asm) => { const log: Sarif.Log = { @@ -41,6 +42,7 @@ export async function sarifParser(interpretedResultsPath: string): Promise Date: Tue, 11 Oct 2022 16:16:29 -0700 Subject: [PATCH 07/30] Reset test timeout to 2 min --- .../src/vscode-tests/no-workspace/query-results.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 984d96ac256..90926c98e6d 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -156,8 +156,8 @@ describe('query-results', () => { }); it('should interpretResultsSarif', async function() { - // up to 5 minutes per test - this.timeout(5 * 60 * 1000); + // up to 2 minutes per test + this.timeout(2 * 60 * 1000); const spy = sandbox.mock(); spy.returns({ a: '1234' }); From fc32971d6d2f618bc6c042d3b219027b8521daa3 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Tue, 11 Oct 2022 16:16:51 -0700 Subject: [PATCH 08/30] Add tests for valid and invalid small SARIF files --- .../no-workspace/query-results.test.ts | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 90926c98e6d..6a1b3668feb 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -206,10 +206,10 @@ describe('query-results', () => { resultsPath, interpretedResultsPath, sourceInfo ); - // try a third time, but this time we get from file + // try a third time, but this time we get from a valid small SARIF file spy.reset(); fs.writeFileSync(interpretedResultsPath, JSON.stringify({ - a: 6 + runs: [{ results: [] }] // A run needs results to succeed. }), 'utf8'); const results3 = await interpretResultsSarif( mockServer, @@ -219,7 +219,34 @@ describe('query-results', () => { }, sourceInfo as SourceInfo ); - expect(results3).to.deep.eq({ a: 6, t: 'SarifInterpretationData' }); + // We do not re-interpret if we are reading from a SARIF file. + expect(spy).to.not.have.been.called; + + expect(results3).to.have.property('t', 'SarifInterpretationData'); + expect(results3).to.have.nested.property('runs[0].results'); + + // try a fourth time, but this time we use an invalid small SARIF file + spy.reset(); + fs.writeFileSync(interpretedResultsPath, JSON.stringify({ + a: '6' // Invalid: no runs or results + }), 'utf8'); + + await expect( + interpretResultsSarif( + mockServer, + metadata, + { + resultsPath, interpretedResultsPath + }, + sourceInfo as SourceInfo) + ).to.be.rejectedWith('Parsing output of interpretation failed: Invalid SARIF file: expecting at least one run with result.'); + + // We do not attempt to re-interpret if we are reading from a SARIF file. + expect(spy).to.not.have.been.called; + + // TODO(angelapwen): Try with a valid large SARIF file + + // TODO(angelapwen): Try with a valid large SARIF file }); describe('splat and slurp', () => { From ca8f930f887cb149c276c32561aedae873e62ff0 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Wed, 12 Oct 2022 16:05:03 -0700 Subject: [PATCH 09/30] Add large SARIF file tests --- .../no-workspace/query-results.test.ts | 51 ++++++++++++++++++- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 6a1b3668feb..202b213f3d5 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -244,9 +244,56 @@ describe('query-results', () => { // We do not attempt to re-interpret if we are reading from a SARIF file. expect(spy).to.not.have.been.called; - // TODO(angelapwen): Try with a valid large SARIF file + // Try a fifth time with a valid large SARIF file + spy.reset(); + const validSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' }); + + validSarifStream.write(JSON.stringify({ + runs: [{ results: [] }] // A run needs results to succeed. + }), 'utf8'); + + for (let i = 0; i < 10000000; i++) { + validSarifStream.write(JSON.stringify({ + a: '6' + }), 'utf8'); + } + validSarifStream.end(); + + const results5 = await interpretResultsSarif( + mockServer, + metadata, + { + resultsPath, interpretedResultsPath + }, + sourceInfo as SourceInfo + ); + // We do not re-interpret if we are reading from a SARIF file. + expect(spy).to.not.have.been.called; - // TODO(angelapwen): Try with a valid large SARIF file + expect(results5).to.have.property('t', 'SarifInterpretationData'); + expect(results5).to.have.nested.property('runs[0].results'); + + // Try a sixth time with an invalid large SARIF file + spy.reset(); + const invalidSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' }); + for (let i = 0; i < 10000000; i++) { + invalidSarifStream.write(JSON.stringify({ + a: '6' + }), 'utf8'); + } + + await expect( + interpretResultsSarif( + mockServer, + metadata, + { + resultsPath, interpretedResultsPath + }, + sourceInfo as SourceInfo) + ).to.be.rejectedWith('Parsing output of interpretation failed: Invalid SARIF file: expecting at least one run with result.'); + + // We do not attempt to re-interpret if we are reading from a SARIF file. + expect(spy).to.not.have.been.called; }); describe('splat and slurp', () => { From 8c92a1933b092e0e1e0baf202ebc74036c043d2c Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Thu, 13 Oct 2022 12:09:43 -0400 Subject: [PATCH 10/30] Delete large valid SARIF before invalid test --- .../src/vscode-tests/no-workspace/query-results.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 202b213f3d5..9be9ad27843 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -273,6 +273,13 @@ describe('query-results', () => { expect(results5).to.have.property('t', 'SarifInterpretationData'); expect(results5).to.have.nested.property('runs[0].results'); + // Explicitly delete the large SARIF file — overwriting causes odd errors. + fs.unlink(interpretedResultsPath, (err) => { + if (err) { + throw err; + } + }); + // Try a sixth time with an invalid large SARIF file spy.reset(); const invalidSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' }); From 438607d5a0f0c5b11a2aec971e0417aa9e917356 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Thu, 13 Oct 2022 12:36:20 -0400 Subject: [PATCH 11/30] Use `del` rather than `unlink` --- .../src/vscode-tests/no-workspace/query-results.test.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 9be9ad27843..9c43accd92d 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -1,4 +1,5 @@ import { expect } from 'chai'; +import * as del from 'del'; import * as path from 'path'; import * as fs from 'fs-extra'; import * as sinon from 'sinon'; @@ -274,11 +275,7 @@ describe('query-results', () => { expect(results5).to.have.nested.property('runs[0].results'); // Explicitly delete the large SARIF file — overwriting causes odd errors. - fs.unlink(interpretedResultsPath, (err) => { - if (err) { - throw err; - } - }); + await del(interpretedResultsPath); // Try a sixth time with an invalid large SARIF file spy.reset(); From a07e549622f312ca4d6ed5499f7c4ab11510b097 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Thu, 13 Oct 2022 13:36:33 -0400 Subject: [PATCH 12/30] Update imports --- extensions/ql-vscode/src/sarif-parser.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/extensions/ql-vscode/src/sarif-parser.ts b/extensions/ql-vscode/src/sarif-parser.ts index 3712ee73b3d..7f81527eac5 100644 --- a/extensions/ql-vscode/src/sarif-parser.ts +++ b/extensions/ql-vscode/src/sarif-parser.ts @@ -1,18 +1,18 @@ import * as Sarif from 'sarif'; import * as fs from 'fs-extra'; -import Assembler = require('stream-json/Assembler'); +import { connectTo } from 'stream-json/Assembler'; import { getErrorMessage } from './pure/helpers-pure'; -import Pick = require('stream-json/filters/Pick'); +import { withParser } from 'stream-json/filters/Pick'; const DUMMY_TOOL: Sarif.Tool = { driver: { name: '' } }; export async function sarifParser(interpretedResultsPath: string): Promise { try { // Parse the SARIF file into token streams, filtering out only the results array. - const pipeline = fs.createReadStream(interpretedResultsPath).pipe(Pick.withParser({ filter: 'runs.0.results' })); + const pipeline = fs.createReadStream(interpretedResultsPath).pipe(withParser({ filter: 'runs.0.results' })); // Creates JavaScript objects from the token stream - const asm = Assembler.connectTo(pipeline); + const asm = connectTo(pipeline); // Returns a constructed Log object with the results of an empty array if no results were found. // If the parser fails for any reason, it will reject the promise. From 9fe8fd87614fd2abf329621a02b638fec2185800 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Thu, 13 Oct 2022 16:14:21 -0400 Subject: [PATCH 13/30] Close write stream --- .../src/vscode-tests/no-workspace/query-results.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 9c43accd92d..182e8f538d4 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -285,6 +285,7 @@ describe('query-results', () => { a: '6' }), 'utf8'); } + invalidSarifStream.end(); await expect( interpretResultsSarif( From d656db037a29a4b40be27e6080932577211117e4 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Wed, 19 Oct 2022 15:20:54 -0700 Subject: [PATCH 14/30] Improve SARIF parser and interpreter tests --- .../no-workspace/query-results.test.ts | 265 ++++++++++-------- 1 file changed, 155 insertions(+), 110 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 182e8f538d4..e4d574c6a56 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -12,6 +12,7 @@ import { tmpDir } from '../../helpers'; import { slurpQueryHistory, splatQueryHistory } from '../../query-serialization'; import { formatLegacyMessage, QueryInProgress } from '../../legacy-query-server/run-queries'; import { EvaluationResult, QueryResultType } from '../../pure/legacy-messages'; +import Sinon = require('sinon'); describe('query-results', () => { let disposeSpy: sinon.SinonSpy; @@ -156,149 +157,193 @@ describe('query-results', () => { }); }); - it('should interpretResultsSarif', async function() { - // up to 2 minutes per test - this.timeout(2 * 60 * 1000); - - const spy = sandbox.mock(); - spy.returns({ a: '1234' }); - const mockServer = { - interpretBqrsSarif: spy - } as unknown as CodeQLCliServer; - - const interpretedResultsPath = path.join(tmpDir.name, 'interpreted.json'); - const resultsPath = '123'; - const sourceInfo = {}; + describe('interpretResultsSarif', () => { + let mockServer: CodeQLCliServer; + let spy: Sinon.SinonExpectation; const metadata = { kind: 'my-kind', id: 'my-id' as string | undefined, scored: undefined }; - const results1 = await interpretResultsSarif( - mockServer, - metadata, - { - resultsPath, interpretedResultsPath - }, - sourceInfo as SourceInfo - ); + const resultsPath = '123'; + const interpretedResultsPath = path.join(tmpDir.name, 'interpreted.json'); + const sourceInfo = {}; - expect(results1).to.deep.eq({ a: '1234', t: 'SarifInterpretationData' }); - expect(spy).to.have.been.calledWith( - metadata, - resultsPath, interpretedResultsPath, sourceInfo - ); + beforeEach(() => { + spy = sandbox.mock(); + spy.returns({ a: '1234' }); - // Try again, but with no id - spy.reset(); - spy.returns({ a: '1234' }); - delete metadata.id; - const results2 = await interpretResultsSarif( - mockServer, - metadata, - { - resultsPath, interpretedResultsPath - }, - sourceInfo as SourceInfo - ); - expect(results2).to.deep.eq({ a: '1234', t: 'SarifInterpretationData' }); - expect(spy).to.have.been.calledWith( - { kind: 'my-kind', id: 'dummy-id', scored: undefined }, - resultsPath, interpretedResultsPath, sourceInfo - ); + mockServer = { + interpretBqrsSarif: spy + } as unknown as CodeQLCliServer; + }); - // try a third time, but this time we get from a valid small SARIF file - spy.reset(); - fs.writeFileSync(interpretedResultsPath, JSON.stringify({ - runs: [{ results: [] }] // A run needs results to succeed. - }), 'utf8'); - const results3 = await interpretResultsSarif( - mockServer, - metadata, - { - resultsPath, interpretedResultsPath - }, - sourceInfo as SourceInfo - ); - // We do not re-interpret if we are reading from a SARIF file. - expect(spy).to.not.have.been.called; + afterEach(async () => { + sandbox.restore(); + await del(interpretedResultsPath); + }); - expect(results3).to.have.property('t', 'SarifInterpretationData'); - expect(results3).to.have.nested.property('runs[0].results'); + it('should interpretResultsSarif', async function() { + // up to 2 minutes per test + this.timeout(2 * 60 * 1000); + + const results = await interpretResultsSarif( + mockServer, + metadata, + { + resultsPath, interpretedResultsPath + }, + sourceInfo as SourceInfo + ); + + expect(results).to.deep.eq({ a: '1234', t: 'SarifInterpretationData' }); + expect(spy).to.have.been.calledWith( + metadata, + resultsPath, interpretedResultsPath, sourceInfo + ); + }); - // try a fourth time, but this time we use an invalid small SARIF file - spy.reset(); - fs.writeFileSync(interpretedResultsPath, JSON.stringify({ - a: '6' // Invalid: no runs or results - }), 'utf8'); + it('should interpretBqrsSarif without ID', async function() { + // up to 2 minutes per test + this.timeout(2 * 60 * 1000); - await expect( - interpretResultsSarif( + delete metadata.id; + const results = await interpretResultsSarif( mockServer, metadata, { resultsPath, interpretedResultsPath }, - sourceInfo as SourceInfo) - ).to.be.rejectedWith('Parsing output of interpretation failed: Invalid SARIF file: expecting at least one run with result.'); + sourceInfo as SourceInfo + ); + expect(results).to.deep.eq({ a: '1234', t: 'SarifInterpretationData' }); + expect(spy).to.have.been.calledWith( + { kind: 'my-kind', id: 'dummy-id', scored: undefined }, + resultsPath, interpretedResultsPath, sourceInfo + ); + }); + + it('should use sarifParser on a valid small SARIF file', async function() { + // up to 2 minutes per test + this.timeout(2 * 60 * 1000); - // We do not attempt to re-interpret if we are reading from a SARIF file. - expect(spy).to.not.have.been.called; + fs.writeFileSync(interpretedResultsPath, JSON.stringify({ + runs: [{ results: [] }] // A run needs results to succeed. + }), 'utf8'); + const results = await interpretResultsSarif( + mockServer, + metadata, + { + resultsPath, interpretedResultsPath + }, + sourceInfo as SourceInfo + ); + // We do not re-interpret if we are reading from a SARIF file. + expect(spy).to.not.have.been.called; - // Try a fifth time with a valid large SARIF file - spy.reset(); - const validSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' }); + expect(results).to.have.property('t', 'SarifInterpretationData'); + expect(results).to.have.nested.property('runs[0].results'); + }); - validSarifStream.write(JSON.stringify({ - runs: [{ results: [] }] // A run needs results to succeed. - }), 'utf8'); + it('should throw an error on an invalid small SARIF file', async function() { + // up to 2 minutes per test + this.timeout(2 * 60 * 1000); - for (let i = 0; i < 10000000; i++) { - validSarifStream.write(JSON.stringify({ - a: '6' + fs.writeFileSync(interpretedResultsPath, JSON.stringify({ + a: '6' // Invalid: no runs or results }), 'utf8'); - } - validSarifStream.end(); - const results5 = await interpretResultsSarif( - mockServer, - metadata, - { - resultsPath, interpretedResultsPath - }, - sourceInfo as SourceInfo - ); - // We do not re-interpret if we are reading from a SARIF file. - expect(spy).to.not.have.been.called; + await expect( + interpretResultsSarif( + mockServer, + metadata, + { + resultsPath, interpretedResultsPath + }, + sourceInfo as SourceInfo) + ).to.be.rejectedWith('Parsing output of interpretation failed: Invalid SARIF file: expecting at least one run with result.'); + + // We do not attempt to re-interpret if we are reading from a SARIF file. + expect(spy).to.not.have.been.called; + }); + + it('should use sarifParser on a valid large SARIF file', async function() { + // up to 2 minutes per test + this.timeout(2 * 60 * 1000); - expect(results5).to.have.property('t', 'SarifInterpretationData'); - expect(results5).to.have.nested.property('runs[0].results'); + const validSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' }); - // Explicitly delete the large SARIF file — overwriting causes odd errors. - await del(interpretedResultsPath); + const finished = new Promise((res, rej) => { + validSarifStream.addListener('finish', res); + validSarifStream.addListener('error', rej); + }); - // Try a sixth time with an invalid large SARIF file - spy.reset(); - const invalidSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' }); - for (let i = 0; i < 10000000; i++) { - invalidSarifStream.write(JSON.stringify({ - a: '6' + validSarifStream.write(JSON.stringify({ + runs: [{ results: [] }] // A run needs results to succeed. }), 'utf8'); - } - invalidSarifStream.end(); - await expect( - interpretResultsSarif( + const iterations = 10_000_000; + for (let i = 0; i < iterations; i++) { + validSarifStream.write(JSON.stringify({ + a: '6' + }), 'utf8'); + } + validSarifStream.end(); + await finished; + + const results = await interpretResultsSarif( mockServer, metadata, { resultsPath, interpretedResultsPath }, - sourceInfo as SourceInfo) - ).to.be.rejectedWith('Parsing output of interpretation failed: Invalid SARIF file: expecting at least one run with result.'); + sourceInfo as SourceInfo + ); + // We do not re-interpret if we are reading from a SARIF file. + expect(spy).to.not.have.been.called; - // We do not attempt to re-interpret if we are reading from a SARIF file. - expect(spy).to.not.have.been.called; + expect(results).to.have.property('t', 'SarifInterpretationData'); + expect(results).to.have.nested.property('runs[0].results'); + }); + + it('should throw an error on an invalid large SARIF file', async function() { + // up to 2 minutes per test + this.timeout(2 * 60 * 1000); + + const invalidSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' }); + + const finished = new Promise((res, rej) => { + invalidSarifStream.addListener('finish', res); + invalidSarifStream.addListener('error', rej); + }); + + invalidSarifStream.write('[', 'utf8'); + const iterations = 10; + for (let i = 0; i < iterations; i++) { + invalidSarifStream.write(JSON.stringify({ + a: '6' + }), 'utf8'); + if (i < iterations - 1) { + invalidSarifStream.write(','); + } + } + invalidSarifStream.write(']', 'utf8'); + invalidSarifStream.end(); + await finished; + + await expect( + interpretResultsSarif( + mockServer, + metadata, + { + resultsPath, interpretedResultsPath + }, + sourceInfo as SourceInfo) + ).to.be.rejectedWith('Parsing output of interpretation failed: Invalid SARIF file: expecting at least one run with result.'); + + // We do not attempt to re-interpret if we are reading from a SARIF file. + expect(spy).to.not.have.been.called; + }); }); describe('splat and slurp', () => { From dcc51cc79c9b20822c13cf65d3db2c99a7586577 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Wed, 19 Oct 2022 15:31:31 -0700 Subject: [PATCH 15/30] Move interpretedResultsPath to before hook --- .../src/vscode-tests/no-workspace/query-results.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index e4d574c6a56..1036c92192c 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -160,18 +160,20 @@ describe('query-results', () => { describe('interpretResultsSarif', () => { let mockServer: CodeQLCliServer; let spy: Sinon.SinonExpectation; + let interpretedResultsPath: string; const metadata = { kind: 'my-kind', id: 'my-id' as string | undefined, scored: undefined }; const resultsPath = '123'; - const interpretedResultsPath = path.join(tmpDir.name, 'interpreted.json'); + const sourceInfo = {}; beforeEach(() => { spy = sandbox.mock(); spy.returns({ a: '1234' }); + interpretedResultsPath = path.join(tmpDir.name, 'interpreted.json'); mockServer = { interpretBqrsSarif: spy From badbed980d5579478b481605c9f95034341f0c40 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Wed, 19 Oct 2022 15:42:59 -0700 Subject: [PATCH 16/30] Use safeDel rather than del --- .../no-workspace/query-results.test.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 1036c92192c..1c7e35798a0 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -1,5 +1,4 @@ import { expect } from 'chai'; -import * as del from 'del'; import * as path from 'path'; import * as fs from 'fs-extra'; import * as sinon from 'sinon'; @@ -160,20 +159,18 @@ describe('query-results', () => { describe('interpretResultsSarif', () => { let mockServer: CodeQLCliServer; let spy: Sinon.SinonExpectation; - let interpretedResultsPath: string; const metadata = { kind: 'my-kind', id: 'my-id' as string | undefined, scored: undefined }; const resultsPath = '123'; - + const interpretedResultsPath = path.join(tmpDir.name, 'interpreted.json'); const sourceInfo = {}; beforeEach(() => { spy = sandbox.mock(); spy.returns({ a: '1234' }); - interpretedResultsPath = path.join(tmpDir.name, 'interpreted.json'); mockServer = { interpretBqrsSarif: spy @@ -182,7 +179,7 @@ describe('query-results', () => { afterEach(async () => { sandbox.restore(); - await del(interpretedResultsPath); + await safeDel(interpretedResultsPath); }); it('should interpretResultsSarif', async function() { @@ -493,4 +490,12 @@ describe('query-results', () => { } return fqi; } + + function safeDel(file: string) { + try { + fs.unlinkSync(file); + } catch (e) { + // ignore + } + } }); From 6bd649a6df2e2b32381c4b0f81684eb1f4c3d35d Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Thu, 20 Oct 2022 08:57:00 -0700 Subject: [PATCH 17/30] Write valid JSON to large valid SARIF test --- .../src/vscode-tests/no-workspace/query-results.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 1c7e35798a0..3747f7c76b5 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -281,12 +281,17 @@ describe('query-results', () => { runs: [{ results: [] }] // A run needs results to succeed. }), 'utf8'); + validSarifStream.write('[', 'utf8'); const iterations = 10_000_000; for (let i = 0; i < iterations; i++) { validSarifStream.write(JSON.stringify({ a: '6' }), 'utf8'); + if (i < iterations - 1) { + validSarifStream.write(','); + } } + validSarifStream.write(']', 'utf8'); validSarifStream.end(); await finished; From 05c6efe3b0bea58f5eaa9bd35f5c366880dab999 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Thu, 20 Oct 2022 08:58:41 -0700 Subject: [PATCH 18/30] Remove file deletion in after block --- .../src/vscode-tests/no-workspace/query-results.test.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 3747f7c76b5..44aeae2e9e9 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -179,7 +179,6 @@ describe('query-results', () => { afterEach(async () => { sandbox.restore(); - await safeDel(interpretedResultsPath); }); it('should interpretResultsSarif', async function() { @@ -495,12 +494,4 @@ describe('query-results', () => { } return fqi; } - - function safeDel(file: string) { - try { - fs.unlinkSync(file); - } catch (e) { - // ignore - } - } }); From f75f0b79e3a699351ddd91f23a89c2dc6c70fba5 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Thu, 20 Oct 2022 13:38:55 -0700 Subject: [PATCH 19/30] Use `safeDel` in after hook --- .../src/vscode-tests/no-workspace/query-results.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 44aeae2e9e9..a05c5d0e14c 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -179,6 +179,7 @@ describe('query-results', () => { afterEach(async () => { sandbox.restore(); + safeDel(interpretedResultsPath); }); it('should interpretResultsSarif', async function() { @@ -430,6 +431,14 @@ describe('query-results', () => { }); }); + function safeDel(file: string) { + try { + fs.unlinkSync(file); + } catch (e) { + // ignore + } + } + function createMockQueryWithResults( queryPath: string, didRunSuccessfully = true, From f7dc7b764db06f9bc17b9e155140ae4a93a96427 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Fri, 21 Oct 2022 10:23:15 -0700 Subject: [PATCH 20/30] Attempt 1000 iterations for large tests --- .../src/vscode-tests/no-workspace/query-results.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index a05c5d0e14c..523f808ade4 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -282,7 +282,7 @@ describe('query-results', () => { }), 'utf8'); validSarifStream.write('[', 'utf8'); - const iterations = 10_000_000; + const iterations = 1000; for (let i = 0; i < iterations; i++) { validSarifStream.write(JSON.stringify({ a: '6' @@ -322,7 +322,7 @@ describe('query-results', () => { }); invalidSarifStream.write('[', 'utf8'); - const iterations = 10; + const iterations = 1000; for (let i = 0; i < iterations; i++) { invalidSarifStream.write(JSON.stringify({ a: '6' From 50b3109687a0c96fe4bb0528d0c09feb0c1e469c Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Fri, 21 Oct 2022 13:09:13 -0700 Subject: [PATCH 21/30] Sleep 10ms in tests on Windows --- .../no-workspace/query-results.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 523f808ade4..84f67d6847c 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -295,6 +295,12 @@ describe('query-results', () => { validSarifStream.end(); await finished; + // We need to sleep to wait for MSFT Defender to scan the file + // so that it can be read by our test. + if (os.platform() === 'win32') { + await sleep(10_000); + } + const results = await interpretResultsSarif( mockServer, metadata, @@ -335,6 +341,12 @@ describe('query-results', () => { invalidSarifStream.end(); await finished; + // We need to sleep to wait for MSFT Defender to scan the file + // so that it can be read by our test. + if (os.platform() === 'win32') { + await sleep(10_000); + } + await expect( interpretResultsSarif( mockServer, @@ -439,6 +451,10 @@ describe('query-results', () => { } } + async function sleep(ms: number) { + return new Promise(resolve => setTimeout(resolve, ms)); + } + function createMockQueryWithResults( queryPath: string, didRunSuccessfully = true, From fbe0b98a2aeb0ef67ebfb6e73b2c0bc56ddbc8b3 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Fri, 21 Oct 2022 13:09:29 -0700 Subject: [PATCH 22/30] Add missing import --- .../src/vscode-tests/no-workspace/query-results.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 84f67d6847c..b63a724f62a 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -1,6 +1,7 @@ import { expect } from 'chai'; import * as path from 'path'; import * as fs from 'fs-extra'; +import * as os from 'os'; import * as sinon from 'sinon'; import { LocalQueryInfo, InitialQueryInfo, interpretResultsSarif } from '../../query-results'; import { QueryWithResults } from '../../run-queries-shared'; From 93bd94c4aa1b609cbd91dcac68e9cb1c3aa41f94 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Fri, 21 Oct 2022 13:09:48 -0700 Subject: [PATCH 23/30] Change stream listeners to close rather than finish --- .../src/vscode-tests/no-workspace/query-results.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index b63a724f62a..d9c1c1fef5d 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -274,7 +274,7 @@ describe('query-results', () => { const validSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' }); const finished = new Promise((res, rej) => { - validSarifStream.addListener('finish', res); + validSarifStream.addListener('close', res); validSarifStream.addListener('error', rej); }); @@ -324,7 +324,7 @@ describe('query-results', () => { const invalidSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' }); const finished = new Promise((res, rej) => { - invalidSarifStream.addListener('finish', res); + invalidSarifStream.addListener('close', res); invalidSarifStream.addListener('error', rej); }); From 86eaf9d87a8728f9d81f64dc4bb28429a265397f Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Fri, 21 Oct 2022 13:10:07 -0700 Subject: [PATCH 24/30] Write 1mil times for SARIF files --- .../src/vscode-tests/no-workspace/query-results.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index d9c1c1fef5d..e27c1c795ca 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -283,7 +283,7 @@ describe('query-results', () => { }), 'utf8'); validSarifStream.write('[', 'utf8'); - const iterations = 1000; + const iterations = 1_000_000; for (let i = 0; i < iterations; i++) { validSarifStream.write(JSON.stringify({ a: '6' @@ -329,7 +329,7 @@ describe('query-results', () => { }); invalidSarifStream.write('[', 'utf8'); - const iterations = 1000; + const iterations = 1_000_000; for (let i = 0; i < iterations; i++) { invalidSarifStream.write(JSON.stringify({ a: '6' From b849fa9df10837b1c3e1d123ec989730b9817bdc Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Fri, 21 Oct 2022 13:10:22 -0700 Subject: [PATCH 25/30] Write to a new path for last test --- .../src/vscode-tests/no-workspace/query-results.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index e27c1c795ca..8b01c90cb34 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -321,7 +321,8 @@ describe('query-results', () => { // up to 2 minutes per test this.timeout(2 * 60 * 1000); - const invalidSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' }); + const invalidLargeInterpretedResultsPath = path.join(tmpDir.name, 'interpreted.json'); + const invalidSarifStream = fs.createWriteStream(invalidLargeInterpretedResultsPath, { flags: 'w' }); const finished = new Promise((res, rej) => { invalidSarifStream.addListener('close', res); From 262fbee781ee91bfc422948001932dfe309afbb3 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Fri, 21 Oct 2022 13:20:40 -0700 Subject: [PATCH 26/30] Refactor timeout to before hook --- .../no-workspace/query-results.test.ts | 23 ++++--------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 8b01c90cb34..100fba955ed 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -169,13 +169,16 @@ describe('query-results', () => { const interpretedResultsPath = path.join(tmpDir.name, 'interpreted.json'); const sourceInfo = {}; - beforeEach(() => { + beforeEach(function(done) { spy = sandbox.mock(); spy.returns({ a: '1234' }); mockServer = { interpretBqrsSarif: spy } as unknown as CodeQLCliServer; + + // Time out all following tests in 2 minutes. + setTimeout(done, 2 * 60 * 1000); }); afterEach(async () => { @@ -184,9 +187,6 @@ describe('query-results', () => { }); it('should interpretResultsSarif', async function() { - // up to 2 minutes per test - this.timeout(2 * 60 * 1000); - const results = await interpretResultsSarif( mockServer, metadata, @@ -204,9 +204,6 @@ describe('query-results', () => { }); it('should interpretBqrsSarif without ID', async function() { - // up to 2 minutes per test - this.timeout(2 * 60 * 1000); - delete metadata.id; const results = await interpretResultsSarif( mockServer, @@ -224,9 +221,6 @@ describe('query-results', () => { }); it('should use sarifParser on a valid small SARIF file', async function() { - // up to 2 minutes per test - this.timeout(2 * 60 * 1000); - fs.writeFileSync(interpretedResultsPath, JSON.stringify({ runs: [{ results: [] }] // A run needs results to succeed. }), 'utf8'); @@ -246,9 +240,6 @@ describe('query-results', () => { }); it('should throw an error on an invalid small SARIF file', async function() { - // up to 2 minutes per test - this.timeout(2 * 60 * 1000); - fs.writeFileSync(interpretedResultsPath, JSON.stringify({ a: '6' // Invalid: no runs or results }), 'utf8'); @@ -268,9 +259,6 @@ describe('query-results', () => { }); it('should use sarifParser on a valid large SARIF file', async function() { - // up to 2 minutes per test - this.timeout(2 * 60 * 1000); - const validSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' }); const finished = new Promise((res, rej) => { @@ -318,9 +306,6 @@ describe('query-results', () => { }); it('should throw an error on an invalid large SARIF file', async function() { - // up to 2 minutes per test - this.timeout(2 * 60 * 1000); - const invalidLargeInterpretedResultsPath = path.join(tmpDir.name, 'interpreted.json'); const invalidSarifStream = fs.createWriteStream(invalidLargeInterpretedResultsPath, { flags: 'w' }); From 75c8aa3958e7ea9fc290b6143f49686493e9fc90 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Fri, 21 Oct 2022 13:35:15 -0700 Subject: [PATCH 27/30] Revert "Refactor timeout to before hook" This reverts commit 262fbee781ee91bfc422948001932dfe309afbb3. --- .../no-workspace/query-results.test.ts | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 100fba955ed..8b01c90cb34 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -169,16 +169,13 @@ describe('query-results', () => { const interpretedResultsPath = path.join(tmpDir.name, 'interpreted.json'); const sourceInfo = {}; - beforeEach(function(done) { + beforeEach(() => { spy = sandbox.mock(); spy.returns({ a: '1234' }); mockServer = { interpretBqrsSarif: spy } as unknown as CodeQLCliServer; - - // Time out all following tests in 2 minutes. - setTimeout(done, 2 * 60 * 1000); }); afterEach(async () => { @@ -187,6 +184,9 @@ describe('query-results', () => { }); it('should interpretResultsSarif', async function() { + // up to 2 minutes per test + this.timeout(2 * 60 * 1000); + const results = await interpretResultsSarif( mockServer, metadata, @@ -204,6 +204,9 @@ describe('query-results', () => { }); it('should interpretBqrsSarif without ID', async function() { + // up to 2 minutes per test + this.timeout(2 * 60 * 1000); + delete metadata.id; const results = await interpretResultsSarif( mockServer, @@ -221,6 +224,9 @@ describe('query-results', () => { }); it('should use sarifParser on a valid small SARIF file', async function() { + // up to 2 minutes per test + this.timeout(2 * 60 * 1000); + fs.writeFileSync(interpretedResultsPath, JSON.stringify({ runs: [{ results: [] }] // A run needs results to succeed. }), 'utf8'); @@ -240,6 +246,9 @@ describe('query-results', () => { }); it('should throw an error on an invalid small SARIF file', async function() { + // up to 2 minutes per test + this.timeout(2 * 60 * 1000); + fs.writeFileSync(interpretedResultsPath, JSON.stringify({ a: '6' // Invalid: no runs or results }), 'utf8'); @@ -259,6 +268,9 @@ describe('query-results', () => { }); it('should use sarifParser on a valid large SARIF file', async function() { + // up to 2 minutes per test + this.timeout(2 * 60 * 1000); + const validSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' }); const finished = new Promise((res, rej) => { @@ -306,6 +318,9 @@ describe('query-results', () => { }); it('should throw an error on an invalid large SARIF file', async function() { + // up to 2 minutes per test + this.timeout(2 * 60 * 1000); + const invalidLargeInterpretedResultsPath = path.join(tmpDir.name, 'interpreted.json'); const invalidSarifStream = fs.createWriteStream(invalidLargeInterpretedResultsPath, { flags: 'w' }); From f37bf65fbe979f6656598dce028ffc62b43e823f Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Fri, 21 Oct 2022 15:12:51 -0700 Subject: [PATCH 28/30] Rename invalid results path --- .../src/vscode-tests/no-workspace/query-results.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 8b01c90cb34..08ede220724 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -321,7 +321,7 @@ describe('query-results', () => { // up to 2 minutes per test this.timeout(2 * 60 * 1000); - const invalidLargeInterpretedResultsPath = path.join(tmpDir.name, 'interpreted.json'); + const invalidLargeInterpretedResultsPath = path.join(tmpDir.name, 'interpreted-invalid.json'); const invalidSarifStream = fs.createWriteStream(invalidLargeInterpretedResultsPath, { flags: 'w' }); const finished = new Promise((res, rej) => { From ca0c8632c2020a3ab7c920f4c27521adf2716504 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Mon, 24 Oct 2022 10:06:45 -0700 Subject: [PATCH 29/30] Update results path var name --- .../src/vscode-tests/no-workspace/query-results.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 08ede220724..ef5530e9b01 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -321,8 +321,8 @@ describe('query-results', () => { // up to 2 minutes per test this.timeout(2 * 60 * 1000); - const invalidLargeInterpretedResultsPath = path.join(tmpDir.name, 'interpreted-invalid.json'); - const invalidSarifStream = fs.createWriteStream(invalidLargeInterpretedResultsPath, { flags: 'w' }); + const interpretedResultsPath = path.join(tmpDir.name, 'interpreted-invalid.json'); + const invalidSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' }); const finished = new Promise((res, rej) => { invalidSarifStream.addListener('close', res); From 671f0e242499f5ad61e8c50ee67bdb84c3ac83ee Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Mon, 24 Oct 2022 10:33:37 -0700 Subject: [PATCH 30/30] Add comment explanation --- .../src/vscode-tests/no-workspace/query-results.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index ef5530e9b01..923e0ad538a 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -321,6 +321,8 @@ describe('query-results', () => { // up to 2 minutes per test this.timeout(2 * 60 * 1000); + // There is a problem on Windows where the file at the prior path isn't able + // to be deleted or written to, so we rename the path for this last test. const interpretedResultsPath = path.join(tmpDir.name, 'interpreted-invalid.json'); const invalidSarifStream = fs.createWriteStream(interpretedResultsPath, { flags: 'w' });