From 2345a21e7c399de37ab6bddf3a354ce652bb5785 Mon Sep 17 00:00:00 2001 From: Masafumi Koba <473530+ybiquitous@users.noreply.github.com> Date: Mon, 17 May 2021 20:41:36 +0900 Subject: [PATCH] Refactor `lib/__tests__/standalone-cache.test.js` (#5311) * Refactor `lib/__tests__/standalone-cache.test.js` - Reduce redundant callbacks via `async/await` syntax. (#4881) - Use `fs.existsSync()` instead of `fs.access()`. See - Fix the disabled test. (#5309) - Inline redundant local variables. - Make expectations using `typeof` more accurate. * Reduce local variables by improving `getConfig()` --- lib/__tests__/standalone-cache.test.js | 272 ++++++++++--------------- 1 file changed, 113 insertions(+), 159 deletions(-) diff --git a/lib/__tests__/standalone-cache.test.js b/lib/__tests__/standalone-cache.test.js index 812abf0e43..604e8be028 100644 --- a/lib/__tests__/standalone-cache.test.js +++ b/lib/__tests__/standalone-cache.test.js @@ -1,20 +1,23 @@ 'use strict'; -const hash = require('../utils/hash'); -const path = require('path'); -const standalone = require('../standalone'); -const fixturesPath = path.join(__dirname, 'fixtures'); const fCache = require('file-entry-cache'); +const path = require('path'); +const { promises: fs, existsSync } = require('fs'); + +const hash = require('../utils/hash'); const replaceBackslashes = require('../testUtils/replaceBackslashes'); -const { promises: fs, constants: fsConstants } = require('fs'); +const standalone = require('../standalone'); -const fileExists = (filePath) => - fs.access(filePath, fsConstants.F_OK).then( - () => true, - () => false, - ); +// NOTE: `fs.rm(file, { force: true })` will be available when we drop the support of older Node versions. +// See +const removeFile = async (filePath) => { + if (existsSync(filePath)) { + await fs.unlink(filePath); + } +}; const cwd = process.cwd(); +const fixturesPath = path.join(__dirname, 'fixtures'); const invalidFile = path.join(fixturesPath, 'empty-block.css'); const syntaxErrorFile = path.join(fixturesPath, 'syntax_error.css'); const validFile = path.join(fixturesPath, 'cache', 'valid.css'); @@ -23,147 +26,120 @@ const newFileDest = path.join(fixturesPath, 'cache', 'newFile.css'); // Config object is getting mutated internally. // Return new object of the same structure to // make sure config doesn't change between runs. -function getConfig() { +function getConfig(additional = {}) { return { files: replaceBackslashes(path.join(fixturesPath, 'cache', '*.css')), config: { rules: { 'block-no-empty': true, 'color-no-invalid-hex': true }, }, cache: true, + ...additional, }; } describe('standalone cache', () => { const expectedCacheFilePath = path.join(cwd, '.stylelintcache'); - beforeEach(() => { + beforeEach(async () => { // Initial run to warm up the cache - return standalone(getConfig()); + await standalone(getConfig()); }); - afterEach(() => { + afterEach(async () => { // Clean up after each test case - return Promise.all([ - fs.unlink(expectedCacheFilePath).catch(() => { - // fs.unlink() throws an error if file doesn't exist and it's ok. We just - // want to make sure it's not there for next test. - }), - fs.unlink(newFileDest).catch(() => { - // fs.unlink() throws an error if file doesn't exist and it's ok. We just - // want to make sure it's not there for next test. - }), - ]); + await removeFile(expectedCacheFilePath); + await removeFile(newFileDest); }); - it('cache file is created at $CWD/.stylelintcache', () => { + it('cache file is created at $CWD/.stylelintcache', async () => { // Ensure cache file exists - return fileExists(expectedCacheFilePath).then((isFileExist) => { - expect(Boolean(isFileExist)).toBe(true); + expect(existsSync(expectedCacheFilePath)).toBe(true); - const fileCache = fCache.createFromFile(expectedCacheFilePath); - const { cache } = fileCache; + const { cache } = fCache.createFromFile(expectedCacheFilePath); - // Ensure cache file contains only linted css file - expect(typeof cache.getKey(validFile) === 'object').toBe(true); - expect(typeof cache.getKey(newFileDest) === 'undefined').toBe(true); - }); + // Ensure cache file contains only linted css file + expect(typeof cache.getKey(validFile)).toBe('object'); + expect(cache.getKey(validFile)).toBeTruthy(); + expect(cache.getKey(newFileDest)).toBeUndefined(); }); - it('only changed files are linted', () => { + it('only changed files are linted', async () => { // Add "changed" file - return fs - .copyFile(validFile, newFileDest) - .then(() => { - // Next run should lint only changed files - return standalone(getConfig()); - }) - .then((output) => { - // Ensure only changed files are linted - const isValidFileLinted = Boolean(output.results.find((file) => file.source === validFile)); - const isNewFileLinted = Boolean(output.results.find((file) => file.source === newFileDest)); - - expect(isValidFileLinted).toBe(false); - expect(isNewFileLinted).toBe(true); - - const fileCache = fCache.createFromFile(expectedCacheFilePath); - const { cache } = fileCache; - - expect(typeof cache.getKey(validFile) === 'object').toBe(true); - expect(typeof cache.getKey(newFileDest) === 'object').toBe(true); - }); + await fs.copyFile(validFile, newFileDest); + + // Next run should lint only changed files + const { results } = await standalone(getConfig()); + + // Ensure only changed files are linted + expect(results.some((file) => file.source === validFile)).toBe(false); + expect(results.some((file) => file.source === newFileDest)).toBe(true); + + const { cache } = fCache.createFromFile(expectedCacheFilePath); + + expect(typeof cache.getKey(validFile)).toBe('object'); + expect(cache.getKey(validFile)).toBeTruthy(); + expect(typeof cache.getKey(newFileDest)).toBe('object'); + expect(cache.getKey(newFileDest)).toBeTruthy(); }); - it('all files are linted on config change', () => { - const changedConfig = getConfig(); - - changedConfig.config.rules['block-no-empty'] = false; - - return fs - .copyFile(validFile, newFileDest) - .then(() => { - // All file should be re-linted as config has changed - return standalone(changedConfig); - }) - .then((output) => { - // Ensure all files are re-linted - const isValidFileLinted = Boolean(output.results.find((file) => file.source === validFile)); - const isNewFileLinted = Boolean(output.results.find((file) => file.source === newFileDest)); - - expect(isValidFileLinted).toBe(true); - expect(isNewFileLinted).toBe(true); - }); + it('all files are linted on config change', async () => { + await fs.copyFile(validFile, newFileDest); + + // All file should be re-linted as config has changed + const { results } = await standalone( + getConfig({ + config: { + rules: { 'block-no-empty': false }, + }, + }), + ); + + // Ensure all files are re-linted + expect(results.some((file) => file.source === validFile)).toBe(true); + expect(results.some((file) => file.source === newFileDest)).toBe(true); }); - it('invalid files are not cached', () => { - return fs - .copyFile(invalidFile, newFileDest) - .then(() => { - // Should lint only changed files - return standalone(getConfig()); - }) - .then((output) => { - expect(output.errored).toBe(true); - // Ensure only changed files are linted - const isValidFileLinted = Boolean(output.results.find((file) => file.source === validFile)); - const isInvalidFileLinted = Boolean( - output.results.find((file) => file.source === newFileDest), - ); - - expect(isValidFileLinted).toBe(false); - expect(isInvalidFileLinted).toBe(true); - - const fileCache = fCache.createFromFile(expectedCacheFilePath); - const { cache } = fileCache; - - expect(typeof cache.getKey(validFile) === 'object').toBe(true); - expect(typeof cache.getKey(newFileDest) === 'undefined').toBe(true); - }); + it('invalid files are not cached', async () => { + await fs.copyFile(invalidFile, newFileDest); + + // Should lint only changed files + const { errored, results } = await standalone(getConfig()); + + expect(errored).toBe(true); + + // Ensure only changed files are linted + expect(results.some((file) => file.source === validFile)).toBe(false); + expect(results.some((file) => file.source === newFileDest)).toBe(true); + + const { cache } = fCache.createFromFile(expectedCacheFilePath); + + expect(typeof cache.getKey(validFile)).toBe('object'); + expect(cache.getKey(validFile)).toBeTruthy(); + expect(cache.getKey(newFileDest)).toBeUndefined(); }); - it('files with syntax errors are not cached', () => { - return fs - .copyFile(syntaxErrorFile, newFileDest) - .then(() => { - // Should lint only changed files - return standalone(getConfig()); - }) - .then(() => { - const fileCache = fCache.createFromFile(expectedCacheFilePath); - const { cache } = fileCache; - - expect(typeof cache.getKey(validFile) === 'object').toBe(true); - expect(typeof cache.getKey(newFileDest) === 'undefined').toBe(true); - }); + + it('files with syntax errors are not cached', async () => { + await fs.copyFile(syntaxErrorFile, newFileDest); + + // Should lint only changed files + await standalone(getConfig()); + + const { cache } = fCache.createFromFile(expectedCacheFilePath); + + expect(typeof cache.getKey(validFile)).toBe('object'); + expect(cache.getKey(validFile)).toBeTruthy(); + expect(cache.getKey(newFileDest)).toBeUndefined(); }); + it('cache file is removed when cache is disabled', async () => { - const noCacheConfig = getConfig(); + await standalone(getConfig({ cache: false })); + expect(existsSync(expectedCacheFilePath)).toBe(false); + }); - noCacheConfig.cache = false; + it("cache doesn't do anything if string is passed", async () => { + // Ensure no cache file + await removeFile(expectedCacheFilePath); - await standalone(noCacheConfig); - await expect(fileExists(expectedCacheFilePath)).resolves.toBe(false); - }); - // eslint-disable-next-line jest/no-disabled-tests - it.skip("cache doesn't do anything if string is passed", async () => { const config = { code: '.foo {}', codeFilename: 'foo.css', @@ -178,64 +154,42 @@ describe('standalone cache', () => { const lintResults = await standalone(config); expect(lintResults.errored).toBe(false); - - await expect(fileExists(expectedCacheFilePath)).resolves.toBe(false); + expect(existsSync(expectedCacheFilePath)).toBe(false); }); }); + describe('standalone cache uses cacheLocation', () => { const cacheLocationFile = path.join(fixturesPath, 'cache', '.cachefile'); const cacheLocationDir = path.join(fixturesPath, 'cache'); const expectedCacheFilePath = path.join(cacheLocationDir, `.stylelintcache_${hash(cwd)}`); - afterEach(() => { + afterEach(async () => { // clean up after each test - return Promise.all([ - fs.unlink(expectedCacheFilePath).catch(() => { - // fs.unlink() throws an error if file doesn't exist and it's ok. We just - // want to make sure it's not there for next test. - }), - fs.unlink(cacheLocationFile).catch(() => { - // fs.unlink() throws an error if file doesn't exist and it's ok. We just - // want to make sure it's not there for next test. - }), - ]); + await removeFile(expectedCacheFilePath); + await removeFile(cacheLocationFile); }); - it('cacheLocation is a file', () => { - const config = getConfig(); - config.cacheLocation = cacheLocationFile; + it('cacheLocation is a file', async () => { + await standalone(getConfig({ cacheLocation: cacheLocationFile })); - return standalone(config) - .then(() => { - // Ensure cache file is created - return fileExists(cacheLocationFile); - }) - .then((fileStats) => { - expect(Boolean(fileStats)).toBe(true); + // Ensure cache file is created + expect(existsSync(cacheLocationFile)).toBe(true); - const fileCache = fCache.createFromFile(cacheLocationFile); - const { cache } = fileCache; + const { cache } = fCache.createFromFile(cacheLocationFile); - expect(typeof cache.getKey(validFile) === 'object').toBe(true); - }); + expect(typeof cache.getKey(validFile)).toBe('object'); + expect(cache.getKey(validFile)).toBeTruthy(); }); - it('cacheLocation is a directory', () => { - const config = getConfig(); - config.cacheLocation = cacheLocationDir; + it('cacheLocation is a directory', async () => { + await standalone(getConfig({ cacheLocation: cacheLocationDir })); - return standalone(config) - .then(() => { - return fileExists(expectedCacheFilePath); - }) - .then((cacheFileStats) => { - // Ensure cache file is created - expect(Boolean(cacheFileStats)).toBe(true); + // Ensure cache file is created + expect(existsSync(expectedCacheFilePath)).toBe(true); - const fileCache = fCache.createFromFile(expectedCacheFilePath); - const { cache } = fileCache; + const { cache } = fCache.createFromFile(expectedCacheFilePath); - expect(typeof cache.getKey(validFile) === 'object').toBe(true); - }); + expect(typeof cache.getKey(validFile)).toBe('object'); + expect(cache.getKey(validFile)).toBeTruthy(); }); });