fix(runner): ignore .gitignore if testDir is explicitly configured (m…

…icrosoft#14430) If the tests are in an explicitly configured testDir (either at the global config level or per project) .gitignore filters are not applied. Fixes microsoft#14381
beyondbycyx · May 26, 2022 · b164d82 · b164d82
1 parent 6efb1ec
commit b164d82
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 18 deletions.
diff --git a/packages/playwright-test/src/loader.ts b/packages/playwright-test/src/loader.ts
@@ -239,6 +239,7 @@ export class Loader {
       projectConfig.snapshotDir = path.resolve(this._configDir, projectConfig.snapshotDir);
 
     const testDir = takeFirst(projectConfig.testDir, config.testDir, this._configDir);
+    const respectGitIgnore = !projectConfig.testDir && !config.testDir;
 
     const outputDir = takeFirst(projectConfig.outputDir, config.outputDir, path.join(throwawayArtifactsPath, 'test-results'));
     const snapshotDir = takeFirst(projectConfig.snapshotDir, config.snapshotDir, testDir);
@@ -256,6 +257,7 @@ export class Loader {
       metadata: takeFirst(projectConfig.metadata, config.metadata, undefined),
       name,
       testDir,
+      _respectGitIgnore: respectGitIgnore,
       snapshotDir,
       _screenshotsDir: screenshotsDir,
       testIgnore: takeFirst(projectConfig.testIgnore, config.testIgnore, []),

diff --git a/packages/playwright-test/src/runner.ts b/packages/playwright-test/src/runner.ts
@@ -244,7 +244,7 @@ export class Runner {
 
     const files = new Map<FullProjectInternal, string[]>();
     for (const project of projects) {
-      const allFiles = await collectFiles(project.testDir);
+      const allFiles = await collectFiles(project.testDir, project._respectGitIgnore);
       const testMatch = createFileMatcher(project.testMatch);
       const testIgnore = createFileMatcher(project.testIgnore);
       const extensions = ['.js', '.ts', '.mjs', '.tsx', '.jsx'];
@@ -534,7 +534,7 @@ function filterSuite(suite: Suite, suiteFilter: (suites: Suite) => boolean, test
   suite._entries = suite._entries.filter(e => entries.has(e)); // Preserve the order.
 }
 
-async function collectFiles(testDir: string): Promise<string[]> {
+async function collectFiles(testDir: string, respectGitIgnore: boolean): Promise<string[]> {
   if (!fs.existsSync(testDir))
     return [];
   if (!fs.statSync(testDir).isDirectory())
@@ -574,25 +574,29 @@ async function collectFiles(testDir: string): Promise<string[]> {
     const entries = await readDirAsync(dir, { withFileTypes: true });
     entries.sort((a, b) => a.name.localeCompare(b.name));
 
-    const gitignore = entries.find(e => e.isFile() && e.name === '.gitignore');
-    if (gitignore) {
-      const content = await readFileAsync(path.join(dir, gitignore.name), 'utf8');
-      const newRules: Rule[] = content.split(/\r?\n/).map(s => {
-        s = s.trim();
-        if (!s)
-          return;
-        // Use flipNegate, because we handle negation ourselves.
-        const rule = new minimatch.Minimatch(s, { matchBase: true, dot: true, flipNegate: true }) as any;
-        if (rule.comment)
-          return;
-        rule.dir = dir;
-        return rule;
-      }).filter(rule => !!rule);
-      rules = [...rules, ...newRules];
+    if (respectGitIgnore) {
+      const gitignore = entries.find(e => e.isFile() && e.name === '.gitignore');
+      if (gitignore) {
+        const content = await readFileAsync(path.join(dir, gitignore.name), 'utf8');
+        const newRules: Rule[] = content.split(/\r?\n/).map(s => {
+          s = s.trim();
+          if (!s)
+            return;
+          // Use flipNegate, because we handle negation ourselves.
+          const rule = new minimatch.Minimatch(s, { matchBase: true, dot: true, flipNegate: true }) as any;
+          if (rule.comment)
+            return;
+          rule.dir = dir;
+          return rule;
+        }).filter(rule => !!rule);
+        rules = [...rules, ...newRules];
+      }
     }
 
     for (const entry of entries) {
-      if (entry === gitignore || entry.name === '.' || entry.name === '..')
+      if (entry.name === '.' || entry.name === '..')
+        continue;
+      if (entry.isFile() && entry.name === '.gitignore')
         continue;
       if (entry.isDirectory() && entry.name === 'node_modules')
         continue;

diff --git a/packages/playwright-test/src/types.ts b/packages/playwright-test/src/types.ts
@@ -58,4 +58,5 @@ export interface FullProjectInternal extends FullProjectPublic {
   _fullyParallel: boolean;
   _expect: Project['expect'];
   _screenshotsDir: string;
+  _respectGitIgnore: boolean;
 }
diff --git a/tests/playwright-test/gitignore.spec.ts b/tests/playwright-test/gitignore.spec.ts
@@ -109,3 +109,51 @@ test('should respect negations and comments in .gitignore', async ({ runInlineTe
     '%%dir3/a.spec.js',
   ]);
 });
+
+test('should ignore .gitignore inside globally configured testDir', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'tests/.gitignore': `
+      *.js
+    `,
+    'playwright.config.js': `
+      module.exports = {
+        testDir: './tests',
+      };
+    `,
+    'tests/a.spec.js': `
+      const { test } = pwt;
+      test('pass', ({}) => {});
+    `,
+    'tests/foo/b.spec.js': `
+      const { test } = pwt;
+      test('pass', ({}) => {});
+    `
+  });
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(2);
+});
+
+
+test('should ignore .gitignore inside project testDir', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'tests/.gitignore': `
+      *.js
+    `,
+    'playwright.config.js': `
+      module.exports = { projects: [
+        { testDir: './tests' },
+      ] };
+    `,
+    'tests/a.spec.js': `
+      const { test } = pwt;
+      test('pass', ({}) => {});
+    `,
+    'tests/foo/b.spec.js': `
+      const { test } = pwt;
+      test('pass', ({}) => {});
+    `
+  });
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(2);
+});
+