Add support for coverage files relative to shifter config

[andrewnicols]

Resubmitting this as a new pull request since github does not allow you to re-open requests.
This was #101 before. I've commented there and there is some additional detail not in this pull request.

This functionality is not covered by the build-dir option as suggested in the close comment. build-dir only makes the build relative. It doesn't change the coverage build options which are currently hardcoded as:
'build/' + filename + '/' + filename + '.js'

This means that it is not relative in a recursive build where the modules may be spread over many directories. For example, our module layout is:

2422 moodle:MDL-XXXXX-jsunit-m> find . -name 'yui'
./admin/tool/capability/yui
./admin/tool/installaddon/yui
./backup/util/ui/yui
./blocks/community/yui
./blocks/navigation/yui
./calendar/yui
./course/yui
./enrol/cohort/yui
./enrol/manual/yui
./enrol/yui
./filter/glossary/yui
./grade/grading/yui
./lib/editor/atto/plugins/bold/yui
./lib/editor/atto/plugins/clear/yui
./lib/editor/atto/plugins/html/yui
./lib/editor/atto/plugins/image/yui
./lib/editor/atto/plugins/indent/yui
./lib/editor/atto/plugins/italic/yui
./lib/editor/atto/plugins/link/yui
./lib/editor/atto/plugins/media/yui
./lib/editor/atto/plugins/orderedlist/yui
./lib/editor/atto/plugins/outdent/yui
./lib/editor/atto/plugins/strike/yui
./lib/editor/atto/plugins/title/yui
./lib/editor/atto/plugins/underline/yui
./lib/editor/atto/plugins/unlink/yui
./lib/editor/atto/plugins/unorderedlist/yui
./lib/editor/atto/yui
./lib/form/yui
./lib/yui
./lib/yuilib/3.9.1/build/yui
./mod/assign/yui
./mod/feedback/yui
./mod/quiz/yui
./theme/bootstrapbase/yui

We have src and build directories in all of these locations, but if we were to write tests and attempt to get coverage on them without this fix, the coverage instrumentation from istanbul would specify the source file relative to each yui directory, and not to the project root. For example, we have moodle-core-tooltip which I've been writing tests for:
lib/yui/src/tooltip; which builds into

`````` lib/yui/build/moodle-core-tooltip/moodle-core-tooltip.jsHowever, the coverage __coverage__ variable write the path is:build/moodle-core-tooltip/moodle-core-tooltip.js```
Thus, when we pass it to grover across the entire project, the path is not found because it is incomplete.

The existing build-dir option cannot be used with a recursive build because it was changed to use path.resolve() recently and thus your builds end up in obscure locations not relevant to the src module depending on how you build (e.g. --walk vs from module vs --recursive).

[caridy]

@andrewnicols there are only 3 options here: relative to the source (build.json), relative to the config .shifter.json or relative to CMD. If none of those solves the problem, I don't think the problem belongs to shifter then. In fact, I think we are abusing shifter with all those options, and I plan to trim some of them. For your example, where you have a bunch of mini yui projects distributed somehow in the file system, you should use another tool to take care of the basic build, then use shifter on each individual yui project. Yogi is one of those tools (we can add more options to yogi if needed). Another example is locator + express-yui (e.g: https://github.com/yahoo/express-yui/tree/master/examples/locator-express), which allows you to walk any folder structure and build to a common (app level) build directory using shifter under the hood. At the end of the day, .shifter.json is just a list of arguments for shifter, so you can get your program to provide such as boilerplate. Let me know if that makes sense, so we can close this one.

[andrewnicols]

@caridy, thanks for the reply.

I agree, the number of options for the coverage path should be relatively small, and I also agree that relative to .shifter.json should be one of them - in fact, that's what this issue seeks to add because it's currently not possible.

At present, the only option Shifter currently supports for code coverage naming is relative to the parent directory of build-dir which isn't really any of the options that you listed. If you look at https://github.com/yui/shifter/blob/master/lib/module.js#L394, you'll see that the coverage name is always prefixed with 'build/' regardless of how it's built. Therefore:

• relative to source (build.json) currently doesn't apply - it's actually relative to the parent of the source's parent directory (../../) by default, unless build-dir is specified - in which case it's from the parent of build-dir (../) - either way, it always starts 'build/';
• relative to config .shifter.json doesn't currently exists; and
• relative to CMD (assume you mean CWD?) is weird/difficult because this will change depending on where you run shifter from - e.g. a --walk will have a cwd of src in yui3; and building a single module will have a cwd of src/[modname]. In any case, we don't do either at present.

I'd actually argue that we should always make coverage relative to the config file rather than any of the other options above. Sadly we can't always do so since many projects don't have an explicit config file - in which case, the current method of prefixing 'build/' to the module name would likely be the safest fallback.

For the most part, this wouldn't affect many people as in reality this is already the case.

In the example I give, we don't have a bunch of mini projects; we just don't lay out our code in the same way as you. Our project is designed to be modular with new plugins dropped into the relevant plugin-type directory, and for the most part YUI is a great fit with that. With a modular system, it makes sense to lay out code with plugins in sub-directories and have each plugin entirely self-sufficient. It simply doesn't make sense to have all of your plugin's code in one directory, except the built YUI modules which are somewhere entirely different and shared with every other part of the project. It effectively ceases to make it truly modular and comes with a whole host of issues.

Shifter already supports recursive walking, and I'd argue that this is a worthwhile feature. However, even if it were removed and we were forced to use any of the tools that you suggest, this would not actually help a project such as ours (Moodle) to run code coverage because shifter always names the coverage variable relative to the parent of the build directory regardless of how it's called. Yes, we could generate code coverage on a per plugin-type basis, but not for the project as a whole and this would make it difficult to use and thus reduce it's worth as people would simply not bother.

Shifter is a tool and it's meant to aid development, not to make it harder. It should help to improve code quality, and without this change it would reduce our ability to do so.

I know that Moodle is not your bread-and-butter, but it's a massive open-source project with many users -- with the 87,079 Moodles volunteering their user statistics, we have 73,724,338 recorded users (https://moodle.org/stats/). I'd really would love to try and improve their experience by correctly packing up all of our YUI modules, and unit testing them.

I do agree that there may be a better way of writing this change - perhaps the ability to specify a directory to build relative too which can default to the location of the .shifter.json? This would mean that it could be optionally specified as a CLI argument:

shifter --walk --coverage-root ../

I think that this issue warrants further discussion.

Thanks in advance,

Andrew