diff --git a/History.md b/History.md index cb042f4bbde..18f8d933311 100644 --- a/History.md +++ b/History.md @@ -8,6 +8,12 @@ N/A ### Changes +- Fixed a bug where modules named with `*.app-tests.js` (or `*.tests.js`) + file extensions sometimes could not be imported by the + `meteor.testModule` entry point when running the `meteor test` command + (or `meteor test --full-app`). + [PR #10402](https://github.com/meteor/meteor/pull/10402) + ## v1.8.0.1, 2018-11-23 ### Breaking changes diff --git a/tools/isobuild/package-source.js b/tools/isobuild/package-source.js index b84de6e3078..2ebdb83dc49 100644 --- a/tools/isobuild/package-source.js +++ b/tools/isobuild/package-source.js @@ -22,7 +22,8 @@ import { PackageAPI } from "./package-api.js"; import { TEST_FILENAME_REGEXPS, APP_TEST_FILENAME_REGEXPS, - isTestFilePath } from './test-files.js'; + isTestFilePath, +} from './test-files.js'; import { convert as convertColonsInPath @@ -1102,15 +1103,6 @@ _.extend(PackageSource.prototype, { // Ignore the usual ignorable files. sourceReadOptions.exclude.push(...ignoreFiles); - // Unless we're running tests, ignore all test filenames and if we are, ignore the - // type of file we *aren't* running - if (!global.testCommandMetadata || global.testCommandMetadata.isTest) { - sourceReadOptions.exclude.push(...APP_TEST_FILENAME_REGEXPS); - } - if (!global.testCommandMetadata || global.testCommandMetadata.isAppTest) { - sourceReadOptions.exclude.push(...TEST_FILENAME_REGEXPS); - } - // Read top-level source files, excluding control files that were not // explicitly included. const controlFiles = ['mobile-config.js']; @@ -1121,11 +1113,48 @@ _.extend(PackageSource.prototype, { const anyLevelExcludes = []; - // If we have a meteor.testModule from package.json, then we don't - // need to exclude tests/ directories from the search, because we - // trust meteor.testModule to identify a single test entry point. - if (! testModule) { + if (testModule) { + // If we have a meteor.testModule from package.json, then we don't + // need to exclude tests/ directories or *.tests.js files from the + // search, because we trust meteor.testModule to identify a single + // test entry point. + } else { anyLevelExcludes.push(/^tests\/$/); + + const { + isTest = false, + isAppTest = false, + } = global.testCommandMetadata || {}; + + if (isTest || isAppTest) { + // If we're running `meteor test` without the --full-app option, + // ignore app-test-only files like *.app-tests.js. + if (! isAppTest) { + sourceReadOptions.exclude.push( + ...APP_TEST_FILENAME_REGEXPS, + ); + } + + // If we're running `meteor test` with the --full-app option, + // ignore non-app-test files like *.tests.js. The wisdom of this + // behavior is debatable, since you might want to run non-app + // tests even when you're using the --full-app option, but it's + // legacy behavior at this point, and it doesn't matter if you're + // using meteor.testModule anyway (recommended). + if (! isTest) { + sourceReadOptions.exclude.push( + ...TEST_FILENAME_REGEXPS, + ); + } + + } else { + // If we're not running `meteor test` (and meteor.testModule is + // not defined in package.json), ignore all test files. + sourceReadOptions.exclude.push( + ...APP_TEST_FILENAME_REGEXPS, + ...TEST_FILENAME_REGEXPS, + ); + } } anyLevelExcludes.push( diff --git a/tools/tests/apps/modules/imports/imported.tests.js b/tools/tests/apps/modules/imports/imported.tests.js new file mode 100644 index 00000000000..5db10f22c06 --- /dev/null +++ b/tools/tests/apps/modules/imports/imported.tests.js @@ -0,0 +1,9 @@ +import assert from "assert"; + +export const name = module.id.split("/").pop(); + +describe(name, () => { + it("should be imported", () => { + assert.strictEqual(name, "imported.tests.js"); + }); +}); diff --git a/tools/tests/apps/modules/tests.js b/tools/tests/apps/modules/tests.js index 9d3a17d2742..454b5915ff4 100644 --- a/tools/tests/apps/modules/tests.js +++ b/tools/tests/apps/modules/tests.js @@ -653,3 +653,10 @@ describe("circular package.json resolution chains", () => { ); }); }); + +describe("imported *.tests.js modules", () => { + const { name } = require("./imports/imported.tests.js"); + it("should be properly compiled", () => { + assert.strictEqual(name, "imported.tests.js"); + }); +}); diff --git a/tools/tests/test-modes.js b/tools/tests/test-modes.js index b86d9fb68a9..0aec92cb3e3 100644 --- a/tools/tests/test-modes.js +++ b/tools/tests/test-modes.js @@ -17,17 +17,17 @@ selftest.define("'meteor test --port' accepts/rejects proper values", function ( runAddPackage.expectExit(0); run = s.run("test", "--port", "3700", "--driver-package", "tmeasday:acceptance-test-driver"); - run.waitSecs(120); + run.waitSecs(30); run.match('App running at: http://localhost:3700/'); run.stop(); run = s.run("test", "--port", "127.0.0.1:3700", "--driver-package", "tmeasday:acceptance-test-driver"); - run.waitSecs(120); + run.waitSecs(30); run.match('App running at: http://127.0.0.1:3700/'); run.stop(); run = s.run("test", "--port", "[::]:3700", "--driver-package", "tmeasday:acceptance-test-driver"); - run.waitSecs(120); + run.waitSecs(30); run.match('App running at: http://[::]:3700/'); run.stop(); }); @@ -80,7 +80,7 @@ selftest.define("'meteor test' eagerly loads correct files", () => { // `meteor` should load app files, but not test files or app-test files run = s.run(); - run.waitSecs(120); + run.waitSecs(30); run.match('index.js'); run.stop(); run.forbid('foo.test.js'); @@ -88,7 +88,7 @@ selftest.define("'meteor test' eagerly loads correct files", () => { // `meteor test` should load test files, but not app files or app-test files run = s.run('test', '--driver-package', 'tmeasday:acceptance-test-driver'); - run.waitSecs(120); + run.waitSecs(30); run.match('foo.test.js'); run.stop(); run.forbid('index.js'); @@ -102,9 +102,9 @@ selftest.define("'meteor test' eagerly loads correct files", () => { 'tmeasday:acceptance-test-driver', '--full-app', ); - run.waitSecs(120); + run.waitSecs(30); run.match('foo.app-test.js'); run.match('index.js'); run.stop(); run.forbid('foo.test.js'); -}) +});