From d6f99d3c8c15de8a8844d9dcab964cffab929312 Mon Sep 17 00:00:00 2001 From: Sebastien Date: Wed, 3 Jan 2024 08:19:20 +0100 Subject: [PATCH] fix: apply ignore settings when copying file (#745) Co-authored-by: Mark Mindenhall <1106269+mmindenhall@users.noreply.github.com> --- __tests__/unit/lib/utils/fsHelper.test.ts | 107 +++++++++++++----- .../flowTranslationProcessor.ts | 2 +- src/utils/fsHelper.ts | 25 +++- src/utils/ignoreHelper.ts | 8 +- 4 files changed, 103 insertions(+), 39 deletions(-) diff --git a/__tests__/unit/lib/utils/fsHelper.test.ts b/__tests__/unit/lib/utils/fsHelper.test.ts index 8c2b6c1a..cecdf307 100644 --- a/__tests__/unit/lib/utils/fsHelper.test.ts +++ b/__tests__/unit/lib/utils/fsHelper.test.ts @@ -16,6 +16,10 @@ import { scanExtension, writeFile, } from '../../../../src/utils/fsHelper' +import { + IgnoreHelper, + buildIgnoreHelper, +} from '../../../../src/utils/ignoreHelper' import { getSpawnContent, treatPathSep, @@ -28,6 +32,7 @@ import { import { EOL } from 'os' import { Work } from '../../../../src/types/work' import { Config } from '../../../../src/types/config' +import { Ignore } from 'ignore' jest.mock('fs-extra') jest.mock('../../../../src/utils/gitLfsHelper') @@ -42,7 +47,9 @@ jest.mock('../../../../src/utils/childProcessUtils', () => { treatPathSep: jest.fn(), } }) +jest.mock('../../../../src/utils/ignoreHelper') +const mockBuildIgnoreHelper = jest.mocked(buildIgnoreHelper) const mockedGetStreamContent = jest.mocked(getSpawnContent) const mockedTreatPathSep = jest.mocked(treatPathSep) const mockedStat = jest.mocked(stat) @@ -54,7 +61,7 @@ beforeEach(() => { jest.resetAllMocks() work = getWork() work.config.from = 'pastsha' - work.config.from = 'recentsha' + work.config.to = 'recentsha' }) describe('gitPathSeparatorNormalizer', () => { @@ -135,6 +142,13 @@ describe('readPathFromGit', () => { }) describe('copyFile', () => { + beforeEach(() => { + mockBuildIgnoreHelper.mockResolvedValue({ + globalIgnore: { + ignores: () => false, + } as unknown as Ignore, + } as unknown as IgnoreHelper) + }) describe('when file is already copied', () => { it('should not copy file', async () => { await copyFiles(work.config, 'source/file') @@ -165,25 +179,61 @@ describe('copyFile', () => { }) }) - describe('when source location is empty', () => { - it('should copy file', async () => { + describe('when content is not a git location', () => { + it('should ignore this path', async () => { // Arrange - mockedTreatPathSep.mockReturnValueOnce('source/copyFile') - mockedGetStreamContent.mockResolvedValue(Buffer.from('')) + const sourcePath = 'source/warning' + mockedGetStreamContent.mockRejectedValue( + `fatal: path '${sourcePath}' does not exist in 'HEAD'` + ) // Act - await copyFiles(work.config, 'source/doNotCopy') + await copyFiles(work.config, sourcePath) // Assert expect(getSpawnContent).toBeCalled() - expect(outputFile).toBeCalledWith( - 'output/source/copyFile', - Buffer.from('') - ) + expect(outputFile).not.toBeCalled() }) }) - describe('when source location is not empty', () => { + describe('when path is ignored', () => { + it('should not copy this path', async () => { + // Arrange + mockBuildIgnoreHelper.mockResolvedValue({ + globalIgnore: { + ignores: () => true, + } as unknown as Ignore, + } as unknown as IgnoreHelper) + + // Act + await copyFiles(work.config, 'source/ignored') + + // Assert + expect(getSpawnContent).not.toBeCalled() + expect(outputFile).not.toBeCalled() + }) + }) + + describe('when content should be copied', () => { + describe('when source location is empty', () => { + it('should copy file', async () => { + // Arrange + + mockedTreatPathSep.mockReturnValueOnce('source/copyFile') + mockedGetStreamContent.mockResolvedValue(Buffer.from('')) + + // Act + await copyFiles(work.config, 'source/doNotCopy') + + // Assert + expect(getSpawnContent).toBeCalled() + expect(outputFile).toBeCalledWith( + 'output/source/copyFile', + Buffer.from('') + ) + }) + }) + describe('when content is a folder', () => { it('should copy the folder', async () => { // Arrange @@ -206,22 +256,7 @@ describe('copyFile', () => { expect(treatPathSep).toBeCalledTimes(1) }) }) - describe('when content is not a git location', () => { - it('should ignore this path', async () => { - // Arrange - const sourcePath = 'source/warning' - mockedGetStreamContent.mockRejectedValue( - `fatal: path '${sourcePath}' does not exist in 'HEAD'` - ) - - // Act - await copyFiles(work.config, sourcePath) - // Assert - expect(getSpawnContent).toBeCalled() - expect(outputFile).not.toBeCalled() - }) - }) describe('when content is a file', () => { beforeEach(async () => { // Arrange @@ -549,6 +584,11 @@ describe('pathExists', () => { describe('writeFile', () => { beforeEach(() => { mockedTreatPathSep.mockReturnValue('folder/file') + mockBuildIgnoreHelper.mockResolvedValue({ + globalIgnore: { + ignores: () => false, + } as unknown as Ignore, + } as unknown as IgnoreHelper) }) it.each(['folder/file', 'folder\\file'])( @@ -581,6 +621,21 @@ describe('writeFile', () => { // Assert expect(outputFile).toBeCalledTimes(1) }) + + it('should not copy ignored path', async () => { + // Arrange + mockBuildIgnoreHelper.mockResolvedValue({ + globalIgnore: { + ignores: () => true, + } as unknown as Ignore, + } as unknown as IgnoreHelper) + + // Act + await writeFile('', '', {} as Config) + + // Assert + expect(outputFile).not.toBeCalled() + }) }) describe('dirExists', () => { diff --git a/src/post-processor/flowTranslationProcessor.ts b/src/post-processor/flowTranslationProcessor.ts index 0a2e80fd..8e14dd25 100644 --- a/src/post-processor/flowTranslationProcessor.ts +++ b/src/post-processor/flowTranslationProcessor.ts @@ -64,7 +64,7 @@ export default class FlowTranslationProcessor extends BaseProcessor { for (const translationPath of translationsIterator) { if ( - !ignoreHelper?.globalIgnore?.ignores(translationPath) && + !ignoreHelper.globalIgnore.ignores(translationPath) && !isSubDir(this.config.output, translationPath) ) { await this._parseTranslationFile(translationPath) diff --git a/src/utils/fsHelper.ts b/src/utils/fsHelper.ts index 812a4551..42808d94 100644 --- a/src/utils/fsHelper.ts +++ b/src/utils/fsHelper.ts @@ -1,7 +1,6 @@ 'use strict' -import { readFile as fsReadFile } from 'fs-extra' import { isAbsolute, join, relative } from 'path' -import { outputFile, stat } from 'fs-extra' +import { readFile as fsReadFile, outputFile, stat } from 'fs-extra' import { GIT_COMMAND, GIT_FOLDER, @@ -10,6 +9,7 @@ import { } from './gitConstants' import { EOLRegex, getSpawnContent, treatPathSep } from './childProcessUtils' import { isLFS, getLFSObjectContentPath } from './gitLfsHelper' +import { buildIgnoreHelper } from './ignoreHelper' import { Config } from '../types/config' const FOLDER = 'tree' @@ -21,9 +21,15 @@ const copiedFiles = new Set() const writtenFiles = new Set() export const copyFiles = async (config: Config, src: string) => { - if (copiedFiles.has(src) || writtenFiles.has(src)) return + if (copiedFiles.has(src) || writtenFiles.has(src)) { + return + } copiedFiles.add(src) + const ignoreHelper = await buildIgnoreHelper(config) + if (ignoreHelper.globalIgnore.ignores(src)) { + return + } try { const bufferData: Buffer = await readPathFromGitAsBuffer(src, config) const utf8Data = bufferData?.toString(UTF8_ENCODING) ?? '' @@ -119,11 +125,18 @@ export async function* scan( export const writeFile = async ( path: string, content: string, - { output }: Config + config: Config ) => { - if (writtenFiles.has(path)) return + if (writtenFiles.has(path)) { + return + } writtenFiles.add(path) - await outputFile(join(output, treatPathSep(path)), content) + + const ignoreHelper = await buildIgnoreHelper(config) + if (ignoreHelper.globalIgnore.ignores(path)) { + return + } + await outputFile(join(config.output, treatPathSep(path)), content) } export const isSubDir = (parent: string, dir: string) => { diff --git a/src/utils/ignoreHelper.ts b/src/utils/ignoreHelper.ts index 8b23058b..129def92 100644 --- a/src/utils/ignoreHelper.ts +++ b/src/utils/ignoreHelper.ts @@ -17,7 +17,7 @@ export class IgnoreHelper { protected readonly destructiveIgnore: Ignore ) {} - public keep(line: string) { + public keep(line: string): boolean { const changeType = line.charAt(0) let ignInstance!: Ignore @@ -47,7 +47,7 @@ export const buildIgnoreHelper = async ({ ? await _buildIgnore(ignoreDestructive) : await _buildIgnore(ignore) - await _addDefaultDestructiveIgnore(destructiveIgnore) + destructiveIgnore.add(BASE_DESTRUCTIVE_IGNORE) ignoreInstance = new IgnoreHelper(globalIgnore, destructiveIgnore) } @@ -80,10 +80,6 @@ const _buildIgnore = async (ignorePath: string) => { return ign } -const _addDefaultDestructiveIgnore = async (destructiveIgnore: Ignore) => { - destructiveIgnore.add(BASE_DESTRUCTIVE_IGNORE) -} - export const resetIgnoreInstance = () => { ignoreInstance = null }