From dfe3853f2facb45744be4aa200f4de729541e4ed Mon Sep 17 00:00:00 2001
From: Avital Oliver <avital@thewe.net>
Date: Mon, 22 Feb 2016 04:08:53 -0800
Subject: [PATCH] Refactor test filename matching code

Now, all the code describing which files represent
test files in an app is in `tools/isobuild/app-test-files.js`,
and there is no code duplication.
---
 tools/isobuild/app-test-files.js  | 35 +++++++++++++++++++++++++++++++
 tools/isobuild/compiler-plugin.js |  9 +++-----
 tools/isobuild/package-source.js  | 13 +++++-------
 3 files changed, 43 insertions(+), 14 deletions(-)
 create mode 100644 tools/isobuild/app-test-files.js

diff --git a/tools/isobuild/app-test-files.js b/tools/isobuild/app-test-files.js
new file mode 100644
index 0000000000..55ec8b6e0c
--- /dev/null
+++ b/tools/isobuild/app-test-files.js
@@ -0,0 +1,35 @@
+import _ from 'underscore';
+import { pathSep } from '../fs/files';
+
+export const TEST_FILENAME_REGEXPS = [
+  // "*.test.*" or "*.tests.*"
+  /\.tests?./,
+  // "test.*" or "tests.*"
+  /^tests?./
+];
+
+export const TEST_DIRNAME_REGEXPS = [
+  // a directory exactly named "tests"
+  /^tests\/$/
+];
+
+// Given a path to a file in an app (relative to the app root
+// directory), is this file a test file?
+export function isTestFilePath(path) {
+  const splitPath = path.split(pathSep);
+
+  // Does any path of the path other than the filename match one of
+  // the test dirname forms?
+  const inTestsDir = _.any(
+    _.initial(splitPath),
+    dirname => _.any(
+      TEST_DIRNAME_REGEXPS,
+      regexp => regexp.test(dirname)));
+
+  // Does the filename match one of the test filename forms?
+  const isTestFilename = _.any(
+    TEST_FILENAME_REGEXPS,
+    regexp => regexp.test(_.last(splitPath)));
+
+  return inTestsDir || isTestFilename;
+}
diff --git a/tools/isobuild/compiler-plugin.js b/tools/isobuild/compiler-plugin.js
index 1184b511b1..b66faef11a 100644
--- a/tools/isobuild/compiler-plugin.js
+++ b/tools/isobuild/compiler-plugin.js
@@ -14,6 +14,8 @@ import Fiber from 'fibers';
 import {sourceMapLength} from '../utils/utils.js';
 import ImportScanner from './import-scanner.js';
 
+import { isTestFilePath } from './app-test-files.js';
+
 // This file implements the new compiler plugins added in Meteor 1.2, which are
 // registered with the Plugin.registerCompiler API.
 //
@@ -394,14 +396,9 @@ class ResourceSlot {
     if (global.testCommandMetadata &&
         (global.testCommandMetadata.isUnitTest ||
          global.testCommandMetadata.isIntegrationTest)) {
-      const isTestFile = _.any(splitPath, (comp) =>
-                               /\.tests?\./.test(comp) ||
-                               /^tests?\./.test(comp) ||
-                               /^tests$/.test(comp));
-
       // test files should always be included, if we're running app
       // tests.
-      return isInImports && !isTestFile;
+      return isInImports && !isTestFilePath(this.inputResource.path);
     } else {
       return isInImports;
     }
diff --git a/tools/isobuild/package-source.js b/tools/isobuild/package-source.js
index 6c3b9ff722..a98c52774d 100644
--- a/tools/isobuild/package-source.js
+++ b/tools/isobuild/package-source.js
@@ -17,6 +17,8 @@ var Profile = require('../tool-env/profile.js').Profile;
 
 import SourceArch from './source-arch.js';
 
+import { TEST_FILENAME_REGEXPS, TEST_DIRNAME_REGEXPS, isTestFilePath } from './app-test-files.js';
+
 // XXX: This is a medium-term hack, to avoid having the user set a package name
 // & test-name in package.describe. We will change this in the new control file
 // world in some way.
@@ -1345,12 +1347,7 @@ _.extend(PackageSource.prototype, {
     // If running in unit test mode (`meteor test-app --unit`), all
     // files other than test files should be loaded lazily
     if (global.testCommandMetadata && global.testCommandMetadata.isUnitTest) {
-      const isTestFile = _.any(relPath.split(files.pathSep), (comp) =>
-                               /\.tests?\./.test(comp) ||
-                               /^tests?\./.test(comp) ||
-                               /^tests$/.test(comp));
-
-      if (!isTestFile) {
+      if (!isTestFilePath(relPath)) {
         fileOptions.lazy = true;
       }
     }
@@ -1392,7 +1389,7 @@ _.extend(PackageSource.prototype, {
     // Unless we're running tests, ignore source files with name
     // "*test*.*", "*tests*.*", "test.*", "tests.*"
     if (!global.testCommandMetadata) {
-      sourceReadOptions.exclude.push(/\.tests?\./, /^tests?\./);
+      Array.prototype.push.apply(sourceReadOptions, TEST_FILENAME_REGEXPS);
     }
 
     // Read top-level source files, excluding control files that were not
@@ -1417,7 +1414,7 @@ _.extend(PackageSource.prototype, {
     ];
 
     if (!global.testCommandMetadata) {
-      anyLevelExcludes.push(/^tests\/$/);
+      Array.prototype.push.apply(anyLevelExcludes, TEST_DIRNAME_REGEXPS);
     }
 
     const topLevelExcludes = isApp ? [