changes to task support promises and some changes to run tests on windows

[fschwiet]

The code changes in this pull request aren't ready, but I thought I'd create this in case you want to give feedback earlier.

I wanted to be able to use promises with Jake directly, so I tried adding support for that (see task.js and task_base.js tests related to promises). In the past I've done this with a helper method, but I thought it'd be nice built into Jake. I believe this implementation will work with any promise implementation that follows the original promises spec, as an implementation detail the test uses the Q framework.

There are a bunch of other changes that I needed for other tests to run on my machine (Windows). Some tests I just uncommented because well that was taking a long time. I'm curious if the tests in this branch pass on your machine (particularly the 'exec(..' -> 'exec(node ..' changes).

Maybe after a break I'll be able to get the commented out tests to pass. I put some notes in on some with what assertions I saw if you want to take a look.

[mde]

This looks great -- a couple of things off the top of my head:

First off, could we separate this into two PRs, one for Promises, and one for tests under Windows?

Secondly, what's the rationale for having a 'start' and 'complete' for tasks that don't actually run? There is admittedly some potential ambiguity about tasks that are already completed, but I've come down pretty squarely on the side of those events indicating that the action is actually getting run. If it doesn't actually do anything, it's not really starting or completing. I don't necessarily object to the task emitting some other event in that case, but I don't think start and complete are appropriate.

[fschwiet]

I created separate pull request with just the promise changes. I need the windows changes to run the tests, but they're not ready for primetime. Honestly I'm not sure when I'd get back to the windows changes,

For the start event, I thought it would be better as then the console logging I generate from the events shows everything that happens. Though the action-less tasks do now work, they still show up. Its not a big deal to me though, I'm going to punt on it for now as I don't have good tests to verify it anyway.

I wanted to get the logging code added though, if you didn't mind. It would be activated on the first call to jake.beVerbose(), which would attach event handlers that log to console.log. The output looks like:

Executing <taskName>

 <existing console output of <taskName>

Executing <nextTaskName>  

<etc etc for every task as it runs>

Execution time summary
  verifyNodeVersion (0ms)
  lint (1931ms)
  writeSampleConfig (3ms)
  testClient (5072ms)
  prepareTempDirectory (11ms)
  createTestDatabase (37ms)
  runMigrations (835ms)
  prepareTestDatabase (NaNms)
  compileJadeViews (297ms)
  buildClientBundle (1910ms)
  commonTestPrequisites (0ms)
  testRemaining (5844ms)
  startSmokeServer (633ms)
  testSmoke (5808ms)
  stopSmokeServer (7ms)
  testSmokeAsRegularTest (0ms)
  testSlow (25693ms)
  test (NaNms)
  default (1ms)
  __root__ (0ms)

I need to clean it up a big more, hiding the NaNms results (thats what happens now as some tasks don't have a start event, but I can clean it up without the emit change)

The caveat is I never bothered to write any tests for that tracing code :) and probably wouldn't bother during the port as well, as there are other things I want to move into. Its fairly simple code an optional, so it seems safe enough to add without automated tests.

[mde]

Where is this logging code? I don't see it in any of the commits.

What do you think about 'skip' event for tasks that don't have to run because they're already done?

[fschwiet]

Sorry I didn't mean to imply I had put the logging code out there. I meant to discuss it before doing it. What I use today is included like this:

task = require("./build/jake-util.js").extendTask(task, jake);

And this is jake-util.js:

exports.extendTask = function(task, jake) {

    var taskRuntimes = [];

    var tracedTask = function(name) {
        var result = task.apply(this, arguments);
        var start;

        result.addListener('start', function() {
            console.log("\nExecuting " + name);
            start = new Date().getTime();
        });

        result.addListener('complete', function() {
            taskRuntimes.push({
                    task: name,
                    ms: new Date().getTime() - start
                });
        });

        return result;
    };
    jake.addListener('complete', function() {
        console.log("Execution time summary");
        taskRuntimes.forEach(function(value) {
            console.log("  " + value.task + " (" + value.ms + "ms)");
        });
});

    return tracedTask;
};

[fschwiet]

I'm off to a camping trip soon, and didn't have time to cleanup the logging code as I probably should.

[mde]

I'm guessing a 'skip' event would be helpful for this? (Execution time would be zero anyhow; there's nothing happening. :))

[fschwiet]

Yes a skip event would work.

[mde]

All right, 'skip' is also in master. Let's let this stuff marinate for a while on master before pushing it out.

On Wed, Jul 24, 2013 at 3:21 PM, fschwiet notifications@github.com wrote:

Yes a skip event would work.

— Reply to this email directly or view it on GitHubhttps://github.com/mde/jake/pull/207#issuecomment-21520930 .

[fschwiet]

Sounds good. I'll be out for a bit anyhow. Thanks for the quick turnaround on this stuff.

[fschwiet]

Closing this as I have a separate pull request for the tracing behavior. I also have a mac again, so not worried about running on the tests on Windows.