Environment variables not honored?

[bzuillsmith]

I noticed that grunt-env does not work with the grunt-mocha-test plugin. It would be nice to use environment variables in a way that is compatible with grunt env so we can set the environment in Gruntfile.js.

[pghalliday]

Seems to work as far as I can tell - i just added a test case to prove it (see test/scenarios/gruntEnvIntegration). Maybe you could elaborate with an example of what doesn't work for you.

[bzuillsmith]

Ahah, I took a look at the code. Didn't realize the require was reimplemented. It looks like NODE_PATH is not handled. NODE_PATH is a special environment variable that is a path to local packages. Instead of saying require('lib/my-module') or require('../lib/my-module'), you can use NODE_PATH=lib and require('my-module'). We should add that to the possible paths to resolve modules to.

[pghalliday]

I'm not sure what you mean when you say the require is reimplemented. Are you referring to the require option. This is indeed based on the original mocha command line implementation of --require

program.on('require', function(mod){
  var abs = exists(mod) || exists(mod + '.js');
  if (abs) mod = resolve(mod);
  requires.push(mod);
});

However I don't see how the implementation here would deal with NODE_PATH differently. Are you trying to require a file with the require option that is referenced relative to NODE_PATH?

A concrete example (Gruntfile and other source) would be helpful in analysing this

[bzuillsmith]

Hmm the problem is definitely that when using NODE_PATH with grunt-mocha-test, require does not seem to see the NODE_PATH. Somehow, grunt-mocha-test is blocking it.

Here is an example a test you could run. I tried forking your repo, but couldn't get it running tests on my machine. Don't have time to figure out why.

In a project with these files, grunt will fail on the second test, but NODE_PATH=lib mocha will succeed.

/test/env.test.js

var path = require('path');
require('should');

describe('Environment variables', function() {

    it('should be passed to the program', function() {

        process.env.NODE_PATH.should.eql('lib');

    });

    it('NODE_PATH should allow non-relative require statements', function() {

        var actualPath = require.resolve('mymodule');
        var expectedPath = path.resolve('./lib/mymodule.js');

        actualPath.should.eql(expectedPath);

    });

});

/lib/mymodule.js

module.exports = "it worked!";

/Gruntfile.js

module.exports = function (grunt) {
    'use strict';
    grunt.initConfig({
        env: {
            test: {
                NODE_PATH: 'lib',
            }
        },
        mochaTest: {
            test: {
                options: {
                    reporter: 'spec'
                },
                src: ['test/**/*.js']
            }
        }
    });

    grunt.loadNpmTasks('grunt-env');
    grunt.loadNpmTasks('grunt-mocha-test');

    grunt.registerTask('default', ['env:test', 'mochaTest']);
};

[pghalliday]

ok i can reproduce this now but i think it will take some investigation - don't suppose you have any ideas? I still don't know what you mean by reimplementing require.

[pghalliday]

hmm i think the problem might be that setting the NODE_PATH variable at runtime may not be enough for NodeJS. My guess is that it has to be set before the process is started. I have confirmed this by setting NODE_PATH before running grunt.

As such i'm not sure this is really an issue and more a feature of how NodeJS deals with NODE_PATH (ie. it adds the path on process initialization and does not check it on every call to require)

I have tried setting require.paths instead but it seems that this functionality has been removed from NodeJS:

Error: require.paths is removed. Use node_modules folders, or the NODE_PATH environment variable instead.

Running out of ideas now

[bzuillsmith]

Yeah, I was just ignorant of what your require code was doing. I shouldn't have said "reimplementing require". I had never seen the internals of Node's require or any code like it, so I saw you setting up paths and such and assumed you were somehow reworking it.

So I assume your task runs on the same process as grunt and does not spawn a new one, right? I only ask because I have a grunt-express-server task that handles NODE_PATH fine, but I am thinking maybe that's because it spawns a new process. Just guessing though.

[bzuillsmith]

Yep, I can confirm. You would have to spawn another process using child_process for this to work. With that module you can pass process_env to the child_process and it will treat it as if it were part of the initial environment. I wonder if I could set up mocha in a grunt-contrib-watch task and pass the option spawn: true to start it in a new process? I'll try that in the morning.

[bzuillsmith]

Turns out I had the time tonight. Running this with the grunt-contrib-watch task works fine. It will spawn a new process for each task it runs. I got the desired behavior with these options for the grunt watch task:

'tests': {
    files: ['test/**/*.js'],
    tasks: ['env:test', 'mochaTest'],
    options: {
        // spawn:true  <-- true is the default, just don't set it to false
        atBegin: true
    }
}

It's quite the edge case so I don't mind if you close this without implementing a spawn option. Maybe make a quick note somewhere in the docs and call it closed?

[pghalliday]

I'm quite surprised it works like that in grunt watch. Unless you're also running env:test before running watch.

Maybe I will add a gotchas section to the readme.

Adding a spawn option would be a fundamentally different approach and it may make more sense to just wrap the Mocha executable in a grunt-shell task.

[bzuillsmith]

Yes, i'm running grunt-env before it as well. Yeah, unless it's easy to hook up with grunt.util.spawn, using another grunt task seems like the way to go.

[pghalliday]

It also occurred to me that grunt-shell could also be used to spawn a grunt sub process while setting the environment too. After all you may not always want to use watch

[binarykitchen]

IMO this is bad. Mocha tests must be respawned each time, no matter if they are being watched or not. This to ensure test are executed in independent environments.

Look over at gulp-spawn-mocha, that's the right approach.

[pghalliday]

Generally I agree. You wouldn't want to run your tests more than once in the same node process (this is what led to the clear require cache options - which i dislike a lot). However some use cases rely on running mocha more than once in the same context (eg. collecting coverage data and running multiple reporters). In some ways I think the architecture of mocha itself leads us down this path. Some people even like things being cached between test runs as it speeds them up (this is a very worthy goal when working with unit tests). Overall i agree that it's a code smell and certainly leads to some gotchas.

[binarykitchen]

Yeah, right @pghalliday, depends on the use case. For mine, I have switched to grunt-mocha-cli. All good.