Support baseDir and instrumentFiles options

[zdennis]

This pull adds the baseDir and instrumentFiles options.

baseDir indicates the base directory that the JS will be served out of. This is useful when the JS files are found in a directory hierarchy on the file-system that doesn't match 1:1 with how the files are being served through connect.

instrumentFiles is a set of files (or file globs that grunt knows how to expand). When used these files will be the only ones that code coverage will be performed against.

The example configuration (from a Gruntfile) below shows how this works:

connect: {
  server: {
    options: {
      port: 8000,
      base: 'site/'
    }
  },
},
jasmine: {
  coverage: {
    src: 'site/js/**/*.js',
    options: {
      templateOptions: {
        template: require('grunt-template-jasmine-requirejs'),
        coverage: 'coverage/coverage.json',
        report: 'coverage',
        instrumentFiles: ['site/js/**/*.js', '!site/js/vendor/**/*.js'],
        baseDir: 'site/',
        templateOptions: {
          requireConfigFile: 'site/js/app/core/config.js',
          requireConfig: {
            baseUrl: '.grunt/grunt-contrib-jasmine/site/js'
          }
        }
      },

      // Setting +replace+ to false ensures that grunt-template-jasmine-istanbul
      // will leave alone the path to the files in the generated +outfile+. If this
      // is set to true then the +outfile+ will contain bad references to files that
      // requireJS tries to load.
      replace: false
    }
  }
}

The source files are found in the site/ directory on the file-system but the connect server servers up site/ as root so it's not necessary to keep site/ in the generated src paths when the requireJS config gets read in, modified, and then written back out.

And in the above configuration all JS files in the site/js/vendor/ directory will be excluding from the code coverage report.

[zdennis]

I was looking to update the tests as well to show a working example but I cannot get them to run currently so I omitted them.

[zdennis]

The pull request has been update to include the instrumentFiles option as well. See updated description for more information.

[zdennis]

ping. can this get reviewed and considered to be merged in? Or at least commented that it won't?

[maenu]

I haven't been very verbose the last few weeks, sorry for that. I didn't have internet access at home, and the weather was just too good to not spend the day nearby the river. But I have planned to look at your pull request and the issues to see what has been going on in that time and dive into this project again.

[maenu]

The time goes way too fast... I had time to have a little look on the changes:

instrumentFiles: This seems like a nice addition that others have been mentioning before (though I don't really understand why you'd want to fool yourself with a high coverage by excluding sources).

baseDir: I don't see how this would load any instrumented sources at all. Wouldn't it load just the originals in the current implementation? Anyway, I don't think that a specific option like this lies in the responsibility of this template. But since this template messes with paths, you should have the chance to process the paths after editing. Maybe we could extend the replace option to take a function too that takes the paths to the originals and instrumented sources and returns the path to use. Something like this replace: function (original, instrumented) { return instrumented.substring(4); } would set options.src for the mixed-in template to the processed sources.

[zdennis]

@maenu, thank for taking a look and responding.

instrumentFiles: This seems like a nice addition that others have been mentioning before (though I don't really understand why you'd want to fool yourself with a high coverage by excluding sources).

It's not about fooling anyone. I (and I assume most others) don't want to include 3rd party libraries in coverage reports. I may use lodash but my application's coverage report doesn't need to be responsible for coverage on lodash. Besides, different projects structure themselves differently and having this option puts the decision for that structure into the hands of the project team.

`baseDir: I don't see how this would load any instrumented sources at all.

This is be inadequately named. It doesn't load any instrumented sources. It massages the path to the source files so when they are served through connect that can be successfully loaded.

Say you have a file on disk site/foo.js and in your Gruntfile you configure src to be site/**/*.js.

Now say that your connect server uses site/ as the root. You don't want jasmine/requirejs/spec-runner to try to load site/foo.js. You want to load foo.js because it's actually served from the root directory.

On disk site/**/*.js made sense because that's where the source files are read from (when you determine what files to instrument), but when loading those files through a web-server you may serve them from a different place.

[joseph-jja]

This is exactly what I was looking for, I'm using various 3rd parties like YUI and I don't care about coverage on those files.

[maenu]

@joseph-jja Well, you should include 3rd-party libraries in the vendor option, then they are never instrumented.

[joseph-jja]

@maenu what vendor option? Is this documented?

[maenu]

@joseph-jja I'm speaking of jasmine's vendor option. This template only instruments the files in the src option.

[zdennis]

@maenu, ping. Have you had the time to reconsider the items in this pull request?

[maenu]

@zdennis Snap, your pull request is like 4 months old and I still haven't found the time to inspect it entirely. Maybe I should get a calendar or something ;).

I intend to add an option like instrumentFiles which would support globbing with white- and blacklisting for files to instrument, so #17 should be solved as well. Honestly, I've still not seen a scenario where this would be absolutely necessary, but it has been wished multiple times. Instead of excluding sources directly, my standpoint is that all source files should be included in the coverage report, a low coverage for some files may be acceptable if you have a good explanation for it. If some files are excluded from the coverage report, the report will not show that they were not included and therefore generates a pimped picture of the reality. A compromise would be if we had the instrumentFiles option, we could generate two reports, one with all sources, one without excluded sources. This option also gives a finer grained control over the instrumentation which is something I don't want to deny.

The baseDir option is a different story. In my opinion, it is not in the responsibility of this template to support it. But extending the replace option as in my comment before would be possible, and if you are running a server, you can always redirect requests to the URLs that don't match the file system as in the RequireJS example.

[zdennis]

@zdennis Snap, your pull request is like 4 months old and I still haven't found the time to inspect it entirely. Maybe I should get a calendar or something ;).

No worries, I know how it goes. I have a calendar and all it does is remind me how busy I am. :)

A compromise would be if we had the instrumentFiles option, we could generate two reports, one with all sources, one without excluded sources. This option also gives a finer grained control over the instrumentation which is something I don't want to deny.

This sounds like more work. What if we start with the simpler case and then iterate and add the second report?

The baseDir option is a different story. In my opinion, it is not in the responsibility of this template to support it.

I can get behind this. I like your idea of allowing replace to take a function.

If I remove baseDir and re-submit with the change around replace would that be enough of a start or do you want to wait until you (or another) is able to get instrumentFiles generating two reports?

[maenu]

A compromise would be if we had the instrumentFiles option, we could generate two reports, one with all sources, one without excluded sources. This option also gives a finer grained control over the instrumentation which is something I don't want to deny.

This sounds like more work. What if we start with the simpler case and then iterate and add the second report?

This was meant to be the description of a scenario how one could use the option. I don't think it would make sense to implement the option this way.

If I remove baseDir and re-submit with the change around replace would that be enough ... ?

That would be awesome.