Skip to content

Conversation

@Kanti
Copy link
Member

@Kanti Kanti commented Sep 11, 2025

No description provided.

@Kanti Kanti requested a review from Copilot September 11, 2025 08:13
@Kanti Kanti self-assigned this Sep 11, 2025
@Kanti Kanti merged commit e5181e3 into main Sep 11, 2025
6 checks passed
@Kanti Kanti deleted the globalIgnores branch September 11, 2025 08:14
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces global ignore functionality and improves documentation for the playwright-addon package. It adds the ability to configure global ignore patterns in the Playwright config that apply across all tests, complementing the existing per-test ignore files.

Key changes:

  • Added global ignore support through project metadata configuration
  • Enhanced filtering logic to support both exact matches and regex patterns
  • Improved documentation with global ignore configuration examples and setup instructions

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/functions/snapshotTest.ts Added clock resume call before accessibility check
src/functions/ignoreFile.ts Implemented global ignore functionality with regex support and improved filtering logic
src/functions/consoleLogging.ts Added empty assertion for update mode
src/functions/checkAccessibility.ts Removed commented-out clock resume call
README.md Enhanced documentation with global ignore configuration and setup instructions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +24 to +25
.map((line: string) => line.trim())
.filter((line: string) => line && !line.startsWith('#'));
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

[nitpick] The explicit type annotations on the callback parameters are unnecessary since TypeScript can infer the types from the array context. Consider removing : string from both callbacks for cleaner code.

Suggested change
.map((line: string) => line.trim())
.filter((line: string) => line && !line.startsWith('#'));
.map((line) => line.trim())
.filter((line) => line && !line.startsWith('#'));

Copilot uses AI. Check for mistakes.
Comment on lines +44 to +53
const ignoresSet = new Set([...globalIgnores, ...ignores]);
const allIgnores = [...ignoresSet].map((ignore) => {
if (typeof ignore !== 'string') {
return ignore;
}
if (!ignore.startsWith('^') && !ignore.endsWith('$')) {
return ignore;
}
return new RegExp(ignore);
});
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

Creating a Set from mixed types (string | RegExp) and then converting back to an array defeats the purpose of deduplication, as RegExp objects with identical patterns are different object references. This could lead to duplicate regex patterns in the final array.

Suggested change
const ignoresSet = new Set([...globalIgnores, ...ignores]);
const allIgnores = [...ignoresSet].map((ignore) => {
if (typeof ignore !== 'string') {
return ignore;
}
if (!ignore.startsWith('^') && !ignore.endsWith('$')) {
return ignore;
}
return new RegExp(ignore);
});
// Deduplicate ignores: strings by value, RegExp by source+flags
const seenStrings = new Set<string>();
const seenRegexes = new Set<string>();
const allIgnores: (string | RegExp)[] = [];
for (const ignore of [...globalIgnores, ...ignores]) {
if (typeof ignore === 'string') {
// If string looks like a regex, convert to RegExp
if (ignore.startsWith('^') || ignore.endsWith('$')) {
const regexKey = ignore + '';
if (!seenRegexes.has(regexKey)) {
seenRegexes.add(regexKey);
allIgnores.push(new RegExp(ignore));
}
} else {
if (!seenStrings.has(ignore)) {
seenStrings.add(ignore);
allIgnores.push(ignore);
}
}
} else if (ignore instanceof RegExp) {
const regexKey = ignore.source + '|' + ignore.flags;
if (!seenRegexes.has(regexKey)) {
seenRegexes.add(regexKey);
allIgnores.push(ignore);
}
}
}

Copilot uses AI. Check for mistakes.
const update = ['all', 'changed'].includes(test.info().config.updateSnapshots);
if (update) {
await writeIgnoreFile(logs, '.consoleignore');
expect([], 'no console outputs').toEqual([]);
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

This assertion will always pass since an empty array equals an empty array. This appears to be a placeholder or debugging code that should be removed or replaced with meaningful logic.

Suggested change
expect([], 'no console outputs').toEqual([]);
expect(logs, 'no console outputs').toEqual([]);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant