Skip to content

[WIP] Add tests for .ignore file behavior with customPatterns (#959) #960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
yamadashy wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

[WIP] Add tests for .ignore file behavior with customPatterns (#959) #960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4786c69
test(config): Add tests for .ignore file behavior with customPatterns
yamadashy Nov 23, 2025
4eeb116
[autofix.ci] apply automated fixes
autofix-ci[bot] Nov 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions tests/config/configMerge.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { describe, expect, test } from 'vitest';
import { mergeConfigs } from '../../src/config/configLoad.js';
import type { RepomixConfigCli, RepomixConfigFile } from '../../src/config/configSchema.js';

describe('configMerge', () => {
describe('ignore settings merge', () => {
test('should preserve useDotIgnore when customPatterns is defined in fileConfig', () => {
const cwd = '/test/dir';
const fileConfig: RepomixConfigFile = {
ignore: {
customPatterns: ['bin/'],
},
};
const cliConfig: RepomixConfigCli = {};

const merged = mergeConfigs(cwd, fileConfig, cliConfig);

// useDotIgnore should be true by default and should not be affected by customPatterns
Copy link
Contributor

@claude claude bot Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states this "should not be affected" but that's exactly what we're testing for in #959!

If the bug exists, this assertion might actually fail (which would be correct - exposing the bug). Consider adding an explicit check:

// useDotIgnore should be true by default and should not be affected by customPatterns
expect(merged.ignore.useDotIgnore).toBe(true);

Also, it would be valuable to verify what the fileConfig.ignore object looks like after Zod parsing to confirm whether useDotIgnore is undefined or not present.

expect(merged.ignore.useDotIgnore).toBe(true);
expect(merged.ignore.useGitignore).toBe(true);
expect(merged.ignore.useDefaultPatterns).toBe(true);
expect(merged.ignore.customPatterns).toEqual(['bin/']);
});

test('should preserve all ignore settings when customPatterns is defined', () => {
const cwd = '/test/dir';
const fileConfig: RepomixConfigFile = {
ignore: {
customPatterns: ['bin/', 'tmp/'],
},
};
const cliConfig: RepomixConfigCli = {};

const merged = mergeConfigs(cwd, fileConfig, cliConfig);

expect(merged.ignore).toEqual({
useGitignore: true,
useDotIgnore: true,
useDefaultPatterns: true,
customPatterns: ['bin/', 'tmp/'],
});
});

test('should allow explicitly disabling useDotIgnore', () => {
const cwd = '/test/dir';
const fileConfig: RepomixConfigFile = {
ignore: {
useDotIgnore: false,
customPatterns: ['bin/'],
},
};
const cliConfig: RepomixConfigCli = {};

const merged = mergeConfigs(cwd, fileConfig, cliConfig);

expect(merged.ignore.useDotIgnore).toBe(false);
expect(merged.ignore.useGitignore).toBe(true);
expect(merged.ignore.customPatterns).toEqual(['bin/']);
});

test('should merge customPatterns from all sources', () => {
const cwd = '/test/dir';
const fileConfig: RepomixConfigFile = {
ignore: {
customPatterns: ['from-file/'],
},
};
const cliConfig: RepomixConfigCli = {
ignore: {
customPatterns: ['from-cli/'],
},
};

const merged = mergeConfigs(cwd, fileConfig, cliConfig);

expect(merged.ignore.customPatterns).toEqual(['from-file/', 'from-cli/']);
expect(merged.ignore.useDotIgnore).toBe(true);
});
});
});
42 changes: 42 additions & 0 deletions tests/config/configSchema.zod.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { describe, expect, test } from 'vitest';
import { repomixConfigFileSchema } from '../../src/config/configSchema.js';

describe('configSchema - Zod parsing behavior', () => {
test('should parse config with only customPatterns', () => {
const input = {
ignore: {
customPatterns: ['bin/'],
},
};

const result = repomixConfigFileSchema.parse(input);

// useDotIgnore should not be set when not provided in input
Copy link
Contributor

@claude claude bot Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is revealing the likely root cause of issue #959!

When repomixConfigFileSchema.parse() receives a config with only customPatterns, it returns an object where useDotIgnore is undefined (not present). Then when mergeConfigs() does:

ignore: {
  ...baseConfig.ignore,      // useDotIgnore: true
  ...fileConfig.ignore,      // useDotIgnore: undefined
  ...cliConfig.ignore,
}

The spread of fileConfig.ignore will include useDotIgnore: undefined, which overwrites the default true value.

Recommendation: This is actually the expected Zod behavior - it only includes fields that are explicitly set. The fix should be in mergeConfigs() to handle undefined values properly (see my main review comment for solutions).

expect(result.ignore?.useDotIgnore).toBeUndefined();
expect(result.ignore?.useGitignore).toBeUndefined();
expect(result.ignore?.useDefaultPatterns).toBeUndefined();
expect(result.ignore?.customPatterns).toEqual(['bin/']);
});

test('should parse config with useDotIgnore explicitly set', () => {
const input = {
ignore: {
useDotIgnore: true,
customPatterns: ['bin/'],
},
};

const result = repomixConfigFileSchema.parse(input);

expect(result.ignore?.useDotIgnore).toBe(true);
expect(result.ignore?.customPatterns).toEqual(['bin/']);
});

test('should parse empty config', () => {
const input = {};

const result = repomixConfigFileSchema.parse(input);

expect(result.ignore).toBeUndefined();
});
});
111 changes: 111 additions & 0 deletions tests/core/file/fileSearch.ignore.integration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import * as fs from 'node:fs/promises';
import * as path from 'node:path';
import { afterEach, describe, expect, test } from 'vitest';
import { searchFiles } from '../../../src/core/file/fileSearch.js';
import { createMockConfig } from '../../testing/testUtils.js';

describe('fileSearch - .ignore integration', () => {
const testDir = path.join(process.cwd(), 'tests/fixtures/ignore-test-temp');

afterEach(async () => {
// Clean up test directory
try {
await fs.rm(testDir, { recursive: true, force: true });
} catch {
// Ignore errors
Copy link
Contributor

@claude claude bot Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential test isolation issue: Using a fixed directory name ignore-test-temp could cause conflicts if tests run in parallel or if cleanup fails.

Consider using a unique directory per test run:

const testDir = path.join(process.cwd(), 'tests/fixtures', `ignore-test-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`);

Or use vitest's test context to ensure proper isolation.

}
});

test('should respect .ignore file when customPatterns is defined in config', async () => {
// Setup test directory structure
await fs.mkdir(testDir, { recursive: true });
await fs.mkdir(path.join(testDir, 'bin'), { recursive: true });
await fs.mkdir(path.join(testDir, 'spec', 'data'), { recursive: true });

// Create test files
await fs.writeFile(path.join(testDir, 'bin', 'test.sh'), 'bin content');
await fs.writeFile(path.join(testDir, 'spec', 'data', 'test.txt'), 'test content');
await fs.writeFile(path.join(testDir, 'main.js'), 'main content');

// Create .ignore file
await fs.writeFile(path.join(testDir, '.ignore'), 'spec/data/\n');

// Create config with customPatterns
const config = createMockConfig({
ignore: {
Comment on lines +34 to +35
Copy link
Contributor

@claude claude bot Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical issue: This test doesn't match the bug scenario reported in #959!

The bug report shows users defining ONLY customPatterns:

{
  "ignore": {
    "customPatterns": ["bin/"]
  }
}

But this test explicitly sets all flags including useDotIgnore: true. This could mask the bug if the issue is that useDotIgnore defaults aren't being applied properly when only customPatterns is defined.

Suggested change
const config = createMockConfig({
ignore: {
// Create config with ONLY customPatterns (matching user's scenario in #959)
const fileConfig: RepomixConfigFile = {
ignore: {
customPatterns: ['bin/'],
},
};
const cliConfig: RepomixConfigCli = {};
const config = mergeConfigs(testDir, fileConfig, cliConfig);
// Verify the merge preserved the default useDotIgnore: true
expect(config.ignore.useDotIgnore).toBe(true);

This would test the actual merge behavior and expose whether the bug exists.

useGitignore: false,
useDotIgnore: true,
useDefaultPatterns: false,
customPatterns: ['bin/'],
},
});

const result = await searchFiles(testDir, config);

// Both bin/ (from customPatterns) and spec/data/ (from .ignore) should be excluded
expect(result.filePaths).not.toContain('bin/test.sh');
expect(result.filePaths).not.toContain('spec/data/test.txt');
expect(result.filePaths).toContain('main.js');
expect(result.filePaths).toContain('.ignore');
});

test('should respect .ignore file when customPatterns is empty', async () => {
// Setup test directory structure
await fs.mkdir(testDir, { recursive: true });
await fs.mkdir(path.join(testDir, 'spec', 'data'), { recursive: true });

// Create test files
await fs.writeFile(path.join(testDir, 'spec', 'data', 'test.txt'), 'test content');
await fs.writeFile(path.join(testDir, 'main.js'), 'main content');

// Create .ignore file
await fs.writeFile(path.join(testDir, '.ignore'), 'spec/data/\n');

// Create config without customPatterns
const config = createMockConfig({
ignore: {
useGitignore: false,
useDotIgnore: true,
useDefaultPatterns: false,
customPatterns: [],
},
});

const result = await searchFiles(testDir, config);

// spec/data/ (from .ignore) should be excluded
expect(result.filePaths).not.toContain('spec/data/test.txt');
expect(result.filePaths).toContain('main.js');
expect(result.filePaths).toContain('.ignore');
});

test('should not use .ignore when useDotIgnore is false', async () => {
// Setup test directory structure
await fs.mkdir(testDir, { recursive: true });
await fs.mkdir(path.join(testDir, 'spec', 'data'), { recursive: true });

// Create test files
await fs.writeFile(path.join(testDir, 'spec', 'data', 'test.txt'), 'test content');
await fs.writeFile(path.join(testDir, 'main.js'), 'main content');

// Create .ignore file
await fs.writeFile(path.join(testDir, '.ignore'), 'spec/data/\n');

// Create config with useDotIgnore: false
const config = createMockConfig({
ignore: {
useGitignore: false,
useDotIgnore: false,
useDefaultPatterns: false,
customPatterns: [],
},
});

const result = await searchFiles(testDir, config);

// spec/data/ should NOT be excluded because useDotIgnore is false
expect(result.filePaths).toContain('spec/data/test.txt');
expect(result.filePaths).toContain('main.js');
expect(result.filePaths).toContain('.ignore');
});
});
Loading