Skip to content
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.

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

Support a meteor.testModule section in application package.json files. #9714

Merged
merged 12 commits into from Mar 1, 2018

Conversation

@benjamn
Copy link
Member

@benjamn benjamn commented Mar 1, 2018

Similar to #9690, this PR adds support for a meteor.testModule configuration in application package.json files, allowing application developers complete control over what test files should load when running meteor test to test their applications.

Setting meteor.testModule is a great way to specify test entry points explicitly, rather than relying on Meteor's isTestFilePath heuristics. However, this feature is entirely opt-in, so the old behavior is preserved if you do not configure meteor.testModule.

The syntax is identical to meteor.mainModule, so you can set a different meteor.testModule.{client,server,...} for each platform. If a testModule is not specified for a platform, then Meteor's existing rules about test file paths apply for that platform, as before.

Different testModules for client and server:

"meteor": {
  "testModule": {
    "client": "client/tests.js",
    "server": "server/tests.js"
  }
}

Same testModule for both client and server:

"meteor": {
  "testModule": "tests.js"
}

Server tests without any client tests (note that simply omitting the "client" property would cause tests to behave as if there was no meteor.testModule on the client):

"meteor": {
  "testModule": {
    "client": false,
    "server": "server/tests.js"
  }
}

If a testModule is specified, that module will always be loaded eagerly when running meteor test, in addition to any other modules that load eagerly due to meteor.mainModule or other rules regarding module loading. If you run meteor test without the --full-app option, then no application JS modules other than the testModule (and any modules imported by it) will be loaded eagerly. If you run meteor test --full-app, the meteor.mainModule module(s) will be loaded eagerly in addition to the meteor.testModule module(s).

The module(s) specified by meteor.testModule can import other test modules at runtime, so you can still distribute test files across your codebase; just make sure you import the ones you want to run.

benjamn added 11 commits Mar 1, 2018
Determining if Meteor package files should be loaded lazily or eagerly is
a lot easier than doing so for application files, so we should just do
that separately, to avoid any risk of application directory layout logic
interfering with package behavior.
If meteor.mainModule.{client,server,...} === false, no modules will be
loaded eagerly for that architecture. This is useful if you have an app
with no special app/{client,server} directory structure and you want to
specify an entry point for just the client (or just the server), without
accidentally loading everything on the other architecture.
If config.mainModule.client === false, then mainId should === false.
Setting meteor.testModule is a great way to specify test entry points
explicitly, rather than relying on Meteor's isTestFilePath heuristics:
https://github.com/meteor/meteor/blob/devel/tools/isobuild/test-files.js

The syntax is identical to meteor.mainModule, so you can set an explicit
meteor.testModule.{client,server,...} for each platform. If a testModule
is not specified for a platform, then Meteor's existing rules about test
file paths apply for that platform, as before.

If a testModule is specified, that module will always be loaded eagerly
when running `meteor test`, in addition to any other modules that load
eagerly because of meteor.mainModule or other rules regarding module
loading. If you run `meteor test` without the `--full-app` option, then no
application JS modules other than the testModule (and any modules imported
by it) will be loaded eagerly.
@benjamn benjamn added this to the Release 1.6.2 milestone Mar 1, 2018
[ci skip]
@benjamn benjamn merged commit 2e8b359 into devel Mar 1, 2018
2 checks passed
2 checks passed
CLA Author has signed the Meteor CLA.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
benjamn added a commit that referenced this pull request Mar 1, 2018
In order for Meteor to maintain its commitment to being a
zero-configuration tool, any configuration options that we add must come
pre-configured in the best way possible for newly created apps.

In particular, the default new Meteor app must contain a reasonable
testing story, or else we are signalling to the community that testing is
an afterthought.

With that said, this PR is still a work in progress. I welcome your
feedback on how best to configure the default `meteor create` starter app.

Builds on #9690 and #9714.
@pravdomil
Copy link
Contributor

@pravdomil pravdomil commented Mar 7, 2018

that would be handy also for packages in node_modules, first step in transition from atmosphere packages, have anybody try something like this?

@benjamn
Copy link
Member Author

@benjamn benjamn commented Mar 7, 2018

@pravdomil I haven't been thinking about that recently, but you're right, we could potentially scan node_modules packages to find those that have a meteor section in package.json, and run those entry points on application startup (server) and/or page load (client).

That's another great reason to use Meteor-specific configuration instead of just reusing the main or browser fields in package.json files, since every npm package has those already.

@pravdomil
Copy link
Contributor

@pravdomil pravdomil commented Mar 7, 2018

@benjamn I'm writing plugin that does exactly that, since I want to move my repos to npm, can you give me some hints where to start? Since probably you know the codebase a lot better.

@Floriferous
Copy link
Member

@Floriferous Floriferous commented Aug 7, 2019

Can this be used in an effortless way if you have a lot of test files?

For example, I'd like to be able to import all tests from a few directories, without having a massive import list in this testModule file.

@SimonSimCity
Copy link
Contributor

@SimonSimCity SimonSimCity commented Aug 8, 2019

This snippet should help you. Just put it into a file referenced by this feature:

const glob = require('glob');
const path = require('path');

const projectPath = '/Users/simon/my-project-folder/';

glob.sync(`${projectPath}**/*.test.js`).forEach((file) => {
  require(path.resolve(file));
});

I just used the library glob here because it's easy - I guess you get the clue.

I had to define the full path because tests are run in a built meteor app - somewhere in a temporary directory. It includes only the files referenced in meteor.testModule or directly included there - all other files are stripped away.

EDIT: Once this get's approved you'd not even need to include the full path, but use the environment variable instead: #10666

@Floriferous
Copy link
Member

@Floriferous Floriferous commented Aug 8, 2019

@SimonSimCity Looks great! I thought about using glob but didn't know how to tie things up with require and path like you did!

Unfortunately the projectPath is a blocker as we'd like it to work across developers' machines. I guess we could each add our own paths in some env variable, but that's really hacky, and then there's the CI..

Are you sure there's no combo of glob options that could make this work?

@SimonSimCity
Copy link
Contributor

@SimonSimCity SimonSimCity commented Aug 8, 2019

@Floriferous You're on the wrong track here. It has nothing to do with glob or anything that you could install in your application as extension. The meteor application is compiled into a node-js application which is running somewhere else. It has no reference to the original project folder. That's what #10666 should bring.

You can - however - do this by setting an env variable yourself by calling PROJECT_PATH=$PWD meteor test instead of meteor test (this example is for linux and mac - haven't tried how to do it for windows) - but all this is now way off topic of this PR. If you want to know more, take a look at the libraries I linked in the issue and how they currently work around the limitation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants