diff --git a/lib/FindTests.js b/lib/FindTests.js index 2e883aeb..96767e9f 100644 --- a/lib/FindTests.js +++ b/lib/FindTests.js @@ -1,4 +1,3 @@ -const gracefulFs = require('graceful-fs'); const fs = require('fs'); const { globSync } = require('tinyglobby'); const path = require('path'); @@ -102,72 +101,87 @@ function findAllElmFilesInDir(dir) { /** * @param { Array } testFilePaths * @param { import('./Project').Project } project - * @returns { Promise }>> } + * @returns { Array<{ moduleName: string, possiblyTests: Array }> } */ function findTests(testFilePaths, project) { - return Promise.all( - testFilePaths.map((filePath) => { - const matchingSourceDirs = project.testsSourceDirs.filter((dir) => - filePath.startsWith(`${dir}${path.sep}`) - ); + // According to benchmarking in elm-watch, reading 2048 bytes at a time is the fastest: + // https://github.com/lydell/elm-watch/blob/6e2c2f8cf8f8e03a4886ca68e839d38e76fb7148/src/ImportWalker.ts#L114-L116 + // For elm-test’s use cases, it doesn’t seem to matter as much. + const buffer = Buffer.alloc(2048); + + return testFilePaths.map((filePath) => { + const matchingSourceDirs = project.testsSourceDirs.filter((dir) => + filePath.startsWith(`${dir}${path.sep}`) + ); + + // Tests must be in tests/ or in source-directories – otherwise they won’t + // compile. Elm won’t be able to find imports. + switch (matchingSourceDirs.length) { + case 0: + throw Error( + missingSourceDirectoryError( + filePath, + project.elmJson.type === 'package' + ) + ); - // Tests must be in tests/ or in source-directories – otherwise they won’t - // compile. Elm won’t be able to find imports. - switch (matchingSourceDirs.length) { - case 0: - return Promise.reject( - Error( - missingSourceDirectoryError( - filePath, - project.elmJson.type === 'package' - ) - ) - ); - - case 1: - // Keep going. - break; - - default: - // This shouldn’t be possible for package projects. - return Promise.reject( - new Error( - multipleSourceDirectoriesError( - filePath, - matchingSourceDirs, - project.testsDir - ) - ) - ); - } - - // By finding the module name from the file path we can import it even if - // the file is full of errors. Elm will then report what’s wrong. - const moduleNameParts = path - .relative(matchingSourceDirs[0], filePath) - .replace(/\.elm$/, '') - .split(path.sep); - const moduleName = moduleNameParts.join('.'); - - if (!moduleNameParts.every(Parser.isUpperName)) { - return Promise.reject( - new Error( - badModuleNameError(filePath, matchingSourceDirs[0], moduleName) + case 1: + // Keep going. + break; + + default: + // This shouldn’t be possible for package projects. + throw new Error( + multipleSourceDirectoriesError( + filePath, + matchingSourceDirs, + project.testsDir ) ); - } - - return Parser.extractExposedPossiblyTests( - filePath, - // We’re reading files asynchronously in a loop here, so it makes sense - // to use graceful-fs to avoid “too many open files” errors. - gracefulFs.createReadStream - ).then((possiblyTests) => ({ - moduleName, - possiblyTests, - })); - }) - ); + } + + // By finding the module name from the file path we can import it even if + // the file is full of errors. Elm will then report what’s wrong. + const moduleNameParts = path + .relative(matchingSourceDirs[0], filePath) + .replace(/\.elm$/, '') + .split(path.sep); + const moduleName = moduleNameParts.join('.'); + + if (!moduleNameParts.every(Parser.isUpperName)) { + throw new Error( + badModuleNameError(filePath, matchingSourceDirs[0], moduleName) + ); + } + + const possiblyTests = Parser.extractExposedPossiblyTests( + filePath, + readChunked(buffer, filePath) + ); + + return { + moduleName, + possiblyTests, + }; + }); +} + +/** + * @param { Buffer } buffer + * @param { string } filePath + * @returns { Generator } + */ +function* readChunked(buffer, filePath) { + const handle = fs.openSync(filePath, 'r'); + let bytesRead = 0; + try { + while ((bytesRead = fs.readSync(handle, buffer)) > 0) { + // The last read might contain less bytes than we asked for. + yield buffer.subarray(0, bytesRead); + } + } finally { + fs.closeSync(handle); + } } /** diff --git a/lib/Parser.js b/lib/Parser.js index 62685936..70239425 100644 --- a/lib/Parser.js +++ b/lib/Parser.js @@ -23,211 +23,202 @@ const LOG_ERRORS = 'ELM_TEST_LOG_PARSE_ERRORS' in process.env; * a whole token it feeds it to the parser, which works token by token. Both * parse just enough to be able to extract all exposed names that could be tests * without false positives. - * - * @param { string } filePath - * @param { ( - path: string, - options?: { encoding?: BufferEncoding } - ) => import('stream').Readable & { close(): void } } createReadStream - * @returns { Promise> } + * + * @param { string } filePath + * @param { Generator } readChunked + * @returns { Array } */ -function extractExposedPossiblyTests(filePath, createReadStream) { - return new Promise((resolve, reject) => { - /** @type { Array } */ - const exposedPossiblyTests = []; - /** @type { TokenizerState } */ - let tokenizerState = { tag: 'MaybeNewChunk' }; - /** @type { ParserState } */ - let parserState = { - tag: 'ModuleDeclaration', - lastToken: 'Nothing', - }; - let lastLowerName = ''; - let justSawCR = false; - - const readable = createReadStream(filePath, { - encoding: 'utf8', - }); - readable.on('error', reject); - readable.on('data', onData); - readable.on('close', () => { - // There’s no need to flush here. It can’t result in more exposed names. - resolve(exposedPossiblyTests); - }); - - /** - * @param { string } chunk - * @returns { void } - */ - function onData(chunk) { - for (let index = 0; index < chunk.length; index++) { - const char = chunk[index]; - - if (justSawCR) { - if (char !== '\n') { - // Elm only supports LF and CRLF, not CR by itself. - Promise.resolve(expected('LF', char)) - .then(stop) - .then(resolve, reject); - readable.close(); - break; - } - justSawCR = false; - } else if (char === '\r') { - justSawCR = true; - // Ignore CR so that the rest of the code can handle only LF. - continue; +function extractExposedPossiblyTests(filePath, readChunked) { + /** @type { Array } */ + const exposedPossiblyTests = []; + /** @type { TokenizerState } */ + let tokenizerState = { tag: 'MaybeNewChunk' }; + /** @type { ParserState } */ + let parserState = { + tag: 'ModuleDeclaration', + lastToken: 'Nothing', + }; + let lastLowerName = ''; + let justSawCR = false; + + /** + * @param { Buffer } chunk + * @returns { boolean } + */ + function onData(chunk) { + for (const char of chunk.toString('utf8')) { + if (justSawCR) { + if (char !== '\n') { + // Elm only supports LF and CRLF, not CR by itself. + return stop(expected('LF', char)); } + justSawCR = false; + } else if (char === '\r') { + justSawCR = true; + // Ignore CR so that the rest of the code can handle only LF. + continue; + } - const result = tokenize(char, tokenizerState); - if (!Array.isArray(result)) { - Promise.resolve(result).then(stop).then(resolve, reject); - readable.close(); - break; - } + const result = tokenize(char, tokenizerState); + if (!Array.isArray(result)) { + return stop(result); + } - const [nextTokenizerState, flushCommands] = result; - let seenFlush = false; - let hasFlushed = false; - let flushResult = undefined; - - for (const flushCommand of flushCommands) { - switch (flushCommand.tag) { - case 'Flush': - seenFlush = true; - break; - case 'FlushToken': - if (flushResult === undefined) { - hasFlushed = true; - flushResult = flush(flushCommand.token); - } - break; - default: - unreachable(flushCommand, 'tag'); - } - } + const [nextTokenizerState, flushCommands] = result; + let seenFlush = false; + let hasFlushed = false; + let flushResult = undefined; - if (seenFlush && !hasFlushed && flushResult === undefined) { - flushResult = flush(); + for (const flushCommand of flushCommands) { + switch (flushCommand.tag) { + case 'Flush': + seenFlush = true; + break; + case 'FlushToken': + if (flushResult === undefined) { + hasFlushed = true; + flushResult = flush(flushCommand.token); + } + break; + default: + unreachable(flushCommand, 'tag'); } + } - tokenizerState = nextTokenizerState; + if (seenFlush && !hasFlushed && flushResult === undefined) { + flushResult = flush(); + } - if (flushResult !== undefined) { - Promise.resolve(flushResult).then(stop).then(resolve, reject); - readable.close(); - break; - } + tokenizerState = nextTokenizerState; + + if (flushResult !== undefined) { + return stop(flushResult); } } - /** - * @param { Token } [token] - * @returns { OnParserTokenResult | undefined } - */ - function flush(token) { - if ( - tokenizerState.tag === 'Initial' && - tokenizerState.otherTokenChars !== '' - ) { - const value = tokenizerState.otherTokenChars; - tokenizerState.otherTokenChars = ''; - const error = onParserToken( - isLowerName(value) - ? { tag: 'LowerName', value } - : isUpperName(value) - ? { tag: 'UpperName', value } - : { tag: 'Other', value } - ); - if (error !== undefined) { - return error; - } - } - if (token !== undefined) { - return onParserToken(token); + return false; + } + + /** + * @param { Token } [token] + * @returns { OnParserTokenResult | undefined } + */ + function flush(token) { + if ( + tokenizerState.tag === 'Initial' && + tokenizerState.otherTokenChars !== '' + ) { + const value = tokenizerState.otherTokenChars; + tokenizerState.otherTokenChars = ''; + const error = onParserToken( + isLowerName(value) + ? { tag: 'LowerName', value } + : isUpperName(value) + ? { tag: 'UpperName', value } + : { tag: 'Other', value } + ); + if (error !== undefined) { + return error; } - return undefined; } + if (token !== undefined) { + return onParserToken(token); + } + return undefined; + } - /** - * @param { OnParserTokenResult } result - * @returns { Array } - */ - function stop(result) { - switch (result.tag) { - case 'ParseError': - if (LOG_ERRORS) { - console.error(`${filePath}: ${result.message}`); - } - return []; + /** + * @param { OnParserTokenResult } result + * @returns { true } + */ + function stop(result) { + switch (result.tag) { + case 'ParseError': + if (LOG_ERRORS) { + console.error(`${filePath}: ${result.message}`); + } + exposedPossiblyTests.length = 0; + return true; - case 'CriticalParseError': - throw new Error( - `This file is problematic:\n\n${filePath}\n\n${result.message}` - ); + case 'CriticalParseError': + throw new Error( + `This file is problematic:\n\n${filePath}\n\n${result.message}` + ); - case 'StopParsing': - return exposedPossiblyTests; + case 'StopParsing': + return true; - default: - return unreachable(result); - } + default: + return unreachable(result); } + } - /** - * @param { Token } token - * @returns { OnParserTokenResult | undefined } - */ - function onParserToken(token) { - if (token.tag === 'LowerName') { - lastLowerName = token.value; - } - switch (parserState.tag) { - case 'ModuleDeclaration': { - const result = parseModuleDeclaration(token, parserState.lastToken); - if (typeof result === 'string') { - parserState.lastToken = result; - if (result === 'LowerName') { - exposedPossiblyTests.push(lastLowerName); - } - return undefined; - } - switch (result.tag) { - case 'ParseError': - case 'CriticalParseError': - case 'StopParsing': - return result; - case 'NextParserState': - parserState = { tag: 'Rest', lastToken: 'Initial' }; - return undefined; - default: - unreachable(result); + /** + * @param { Token } token + * @returns { OnParserTokenResult | undefined } + */ + function onParserToken(token) { + if (token.tag === 'LowerName') { + lastLowerName = token.value; + } + switch (parserState.tag) { + case 'ModuleDeclaration': { + const result = parseModuleDeclaration(token, parserState.lastToken); + if (typeof result === 'string') { + parserState.lastToken = result; + if (result === 'LowerName') { + exposedPossiblyTests.push(lastLowerName); } + return undefined; } - - case 'Rest': { - const result = parseRest(token, parserState.lastToken); - if (typeof result === 'string') { - parserState.lastToken = result; - if (result === 'PotentialTestDeclaration=') { - exposedPossiblyTests.push(lastLowerName); - } + switch (result.tag) { + case 'ParseError': + case 'CriticalParseError': + case 'StopParsing': + return result; + case 'NextParserState': + parserState = { tag: 'Rest', lastToken: 'Initial' }; return undefined; + default: + unreachable(result); + } + } + + case 'Rest': { + const result = parseRest(token, parserState.lastToken); + if (typeof result === 'string') { + parserState.lastToken = result; + if (result === 'PotentialTestDeclaration=') { + exposedPossiblyTests.push(lastLowerName); } - switch (result.tag) { - case 'ParseError': - return result; - default: - // Type assertion needed here due to there only being one error variant. - unreachable(/** @type { never } */ (result), 'tag'); - } + return undefined; } + switch (result.tag) { + case 'ParseError': + return result; + default: + // Type assertion needed here due to there only being one error variant. + unreachable(/** @type { never } */ (result), 'tag'); + } + } - default: - unreachable(parserState, 'tag'); + default: + unreachable(parserState, 'tag'); + } + } + + try { + for (const chunk of readChunked) { + if (onData(chunk)) { + break; } } - }); + } finally { + readChunked.return(); + } + + // There’s no need to flush here. It can’t result in more exposed names. + return exposedPossiblyTests; } // First char lowercase: https://github.com/elm/compiler/blob/2860c2e5306cb7093ba28ac7624e8f9eb8cbc867/compiler/src/Parse/Variable.hs#L296-L300 diff --git a/lib/RunTests.js b/lib/RunTests.js index 5d9b8af3..7da7be85 100644 --- a/lib/RunTests.js +++ b/lib/RunTests.js @@ -238,7 +238,7 @@ function runTests( runsExecuted++; const pipeFilename = getPipeFilename(runsExecuted); - const testModules = await FindTests.findTests(testFilePaths, project); + const testModules = FindTests.findTests(testFilePaths, project); const mainModule = Generate.getMainModule(project.generatedCodeDir); const dest = path.join(project.generatedCodeDir, 'elmTestOutput.js'); diff --git a/package-lock.json b/package-lock.json index a0f0f6fa..0240c8a1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,6 @@ "chokidar": "^3.5.3", "commander": "^9.4.1", "elm-solve-deps-wasm": "^1.0.2 || ^2.0.0", - "graceful-fs": "^4.2.10", "split": "^1.0.1", "tinyglobby": "^0.2.10", "which": "^2.0.2", @@ -23,7 +22,6 @@ }, "devDependencies": { "@eslint/js": "9.20.0", - "@types/graceful-fs": "4.1.9", "@types/mocha": "10.0.10", "@types/node": "26.0.1", "@types/split": "1.0.5", @@ -406,16 +404,6 @@ "integrity": "sha512-AYnb1nQyY49te+VRAVgmzfcgjYS91mY5P0TKUDCLEM+gNnA+3T6rWITXRLYCpahpqSQbN5cE+gHpnPyXjHWxcw==", "dev": true }, - "node_modules/@types/graceful-fs": { - "version": "4.1.9", - "resolved": "https://registry.npmjs.org/@types/graceful-fs/-/graceful-fs-4.1.9.tgz", - "integrity": "sha512-olP3sd1qOEe5dXTSaFvQG+02VdRXcdytWLAZsAq1PecU8uqQAhkrnbli7DagjtXKW/Bl7YJbUsa8MPcuc8LHEQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "@types/node": "*" - } - }, "node_modules/@types/http-cache-semantics": { "version": "4.0.4", "resolved": "https://registry.npmjs.org/@types/http-cache-semantics/-/http-cache-semantics-4.0.4.tgz", @@ -1972,7 +1960,8 @@ "node_modules/graceful-fs": { "version": "4.2.10", "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.10.tgz", - "integrity": "sha512-9ByhssR2fPVsNZj478qUUbKfmL0+t5BDVyjShtyZZLiK7ZDAArFFfopyOTj0M05wE2tJPisA4iTnnXl2YoPvOA==" + "integrity": "sha512-9ByhssR2fPVsNZj478qUUbKfmL0+t5BDVyjShtyZZLiK7ZDAArFFfopyOTj0M05wE2tJPisA4iTnnXl2YoPvOA==", + "dev": true }, "node_modules/has-flag": { "version": "4.0.0", @@ -3556,15 +3545,6 @@ "integrity": "sha512-AYnb1nQyY49te+VRAVgmzfcgjYS91mY5P0TKUDCLEM+gNnA+3T6rWITXRLYCpahpqSQbN5cE+gHpnPyXjHWxcw==", "dev": true }, - "@types/graceful-fs": { - "version": "4.1.9", - "resolved": "https://registry.npmjs.org/@types/graceful-fs/-/graceful-fs-4.1.9.tgz", - "integrity": "sha512-olP3sd1qOEe5dXTSaFvQG+02VdRXcdytWLAZsAq1PecU8uqQAhkrnbli7DagjtXKW/Bl7YJbUsa8MPcuc8LHEQ==", - "dev": true, - "requires": { - "@types/node": "*" - } - }, "@types/http-cache-semantics": { "version": "4.0.4", "resolved": "https://registry.npmjs.org/@types/http-cache-semantics/-/http-cache-semantics-4.0.4.tgz", @@ -4563,7 +4543,8 @@ "graceful-fs": { "version": "4.2.10", "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.10.tgz", - "integrity": "sha512-9ByhssR2fPVsNZj478qUUbKfmL0+t5BDVyjShtyZZLiK7ZDAArFFfopyOTj0M05wE2tJPisA4iTnnXl2YoPvOA==" + "integrity": "sha512-9ByhssR2fPVsNZj478qUUbKfmL0+t5BDVyjShtyZZLiK7ZDAArFFfopyOTj0M05wE2tJPisA4iTnnXl2YoPvOA==", + "dev": true }, "has-flag": { "version": "4.0.0", diff --git a/package.json b/package.json index 2b8b2b0c..1475957a 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,6 @@ "chokidar": "^3.5.3", "commander": "^9.4.1", "elm-solve-deps-wasm": "^1.0.2 || ^2.0.0", - "graceful-fs": "^4.2.10", "split": "^1.0.1", "tinyglobby": "^0.2.10", "which": "^2.0.2", @@ -53,7 +52,6 @@ }, "devDependencies": { "@eslint/js": "9.20.0", - "@types/graceful-fs": "4.1.9", "@types/mocha": "10.0.10", "@types/node": "26.0.1", "@types/split": "1.0.5", diff --git a/tests/Parser.js b/tests/Parser.js index 095d99a1..51b779f1 100644 --- a/tests/Parser.js +++ b/tests/Parser.js @@ -1,31 +1,28 @@ 'use strict'; const assert = require('assert'); -const stream = require('stream'); const Parser = require('../lib/Parser'); /** * @param { string } elmCode * @param { Array } expectedExposedNames */ -async function testParser(elmCode, expectedExposedNames) { - const exposed = await Parser.extractExposedPossiblyTests( +function testParser(elmCode, expectedExposedNames) { + const exposed = Parser.extractExposedPossiblyTests( 'SomeFile.elm', - (_, options) => { - const readable = - /** @type { import('stream').Readable & { close(): void } } */ ( - stream.Readable.from([elmCode], { - ...options, - autoDestroy: true, - }) - ); - readable.close = readable.destroy; - return readable; - } + readChunked(elmCode) ); assert.deepStrictEqual(exposed, expectedExposedNames); } +/** + * @param { string } string + * @returns { Generator } + */ +function* readChunked(string) { + yield Buffer.from(string); +} + describe('Parser', () => { describe('valid Elm code', () => { it('handles a basic module definition', () => @@ -210,11 +207,12 @@ One = 1 testParser('module "string" Main exposing (one)', [])); it('treats `effect module` as a critical error', () => - assert.rejects( - testParser( - 'effect module Example where { subscription = MySub } exposing (..)', - ['should', 'not', 'succeed'] - ), + assert.throws( + () => + testParser( + 'effect module Example where { subscription = MySub } exposing (..)', + ['should', 'not', 'succeed'] + ), { message: 'This file is problematic:\n\nSomeFile.elm\n\nIt starts with `effect module`. Effect modules can only exist inside src/ in elm and elm-explorations packages. They cannot contain tests.',