angular-fullstack / generator-angular-fullstack

Yeoman generator for an Angular app with an Express server
https://awk34.gitbook.io/generator-angular-fullstack
6.12k stars 1.24k forks source link

Seeding always occurs when running Mocha, regardless of seedDB #1978

Open mfsjr opened 8 years ago

mfsjr commented 8 years ago

First, thanks for a great generator....

Ok, since angular-fullstack puts test files in different directories, it might make sense to configure mocha to run tests for everything under the 'server' directory.

But, mocha will 'require' all files under that, and merely requiring seed.js will automatically re-seed.

It would seem to be easily fixed by explicitly testing config.seedDB and ignoring all code if false (works for me, so far).

This creates errors when trying to run tests on authenticated endpoints, pretty frustrating actually. I think I saw one or two issues on this but they had been closed.

Thanks, Matt

generator-angular-fullstack | 2.1.1 Node | 0.12.0 npm | 2.14.9 Operating System | OS X 10

Mocha Auth

Awk34 commented 8 years ago

I'm not sure what you mean by mocha will 'require' all files under that. Mocha doesn't require anything.

mfsjr commented 8 years ago

Mocha appears to recursively iterate downward through all subdirectories from the configured test directory, and will load every .js file it finds, see mocha.js starting around line 214:

Mocha.prototype.loadFiles = function(fn) { var self = this; var suite = this.suite; this.files.forEach(function(file) { file = path.resolve(file); suite.emit('pre-require', global, file, self); suite.emit('require', require(file), file, self); suite.emit('post-require', global, file, self); }); fn && fn(); };

Awk34 commented 8 years ago

I should clarify: Mocha doesn't require anything you don't tell it to require.

The loadFiles function isn't doing what you think it's doing. It's just iterating through the this.files array, which is an array of all the files you tell it to require.

So for the server tests, this would be:

[ '.../server/api/thing/index.spec.js',
  '.../server/api/user/index.spec.js',
  '.../server/api/user/user.model.spec.js',
  '.../mocha.global.js' ]

and

[ '.../server/api/thing/thing.integration.js',
  '.../server/api/user/user.integration.js',
  '.../mocha.global.js' ]

for both unit and integration tests, respectively.

For the client, Karma gets files.

If you're not testing through gulp test, etc, something will likely not go as intended

mfsjr commented 8 years ago

Agreed, if you explicitly configure only the test files nothing bad will happen, even though mocha appears to be designed to rip through directories.

But even then, there is a possibility that some other random library might do its own recursive loading, and if it happens in production then you're hung out with a well-known admin user name and password.

Checking the value of config.seedDB inside the file would protect everyone from an unfortunate accident - my two cents.

Thanks, Matt

Awk34 commented 8 years ago

That's not really reasonable. Why would some external library be recursively requiring our files without permission? There should be absolutely no instance of seed.js being required without you explicitly wanting to.

mfsjr commented 8 years ago

Not my intention to be unreasonable...

WebStorm's Mocha test runner offers you the option of configuring to test everything under a directory, and mocha will do it by loading all those files, and it will appear to work. So that's one way it has happened.

Trying to figure out why someone would do something could take a lot of time or just be impossible and leave you with no guarantees, but if there's an easy fix for it then you don't need to.

Awk34 commented 8 years ago

Yeah, I get that that's something you might want to use, but the generator isn't set up to support that. It might be something that could be supported, though, if you want to investigate whether the same functionality as gulp test can be achieved through normal mocha config.