Support source-directories from elm.json #28
Conversation
| @@ -52,10 +52,10 @@ var globalOptions = yargs => | |||
| yargs | |||
| .positional("path", { | |||
| describe: | |||
| "Where are your sources located? This path will be " + | |||
| "Where is your elm.json located? This path will be " + | |||
| @@ -73,10 +77,11 @@ var setupTests = (log, args) => () => { | |||
| "modifyingTests", | |||
| "Generating elm.json for coverage at " + tmpElmJson + "..." | |||
| ); | |||
| var covSrc = path.resolve(path.join(coverageDir, args.path)); | |||
| var originalPath = path.resolve(args.path); | |||
There was a problem hiding this comment.
These two variables were unused
| return sourceDirectories(args) | ||
| .then(sourceDirs => Promise.all(sourceDirs.map(sourceDir => allSourcesFromDir(sourceDir)))) | ||
| .then(sourcesArrays => sourcesArrays.flat()); | ||
| }; |
There was a problem hiding this comment.
This is the new machinery to read all files from all source-directories.
Potential problem: ES6 Array.flat usage in an otherwise quite conservative JavaScript
| var elmJsonPath = path.join(args.path, 'elm.json'); | ||
| return fs.readJson(elmJsonPath) | ||
| .then(elmJson => elmJson['source-directories']); | ||
| }; |
There was a problem hiding this comment.
Reading the elm.json file here
| return path.startsWith('..') | ||
| ? path.replace(/^\.\./,'__dotdot') | ||
| : path; | ||
| } |
There was a problem hiding this comment.
Our code (that I'm hacking elm-coverage for) has this directory structure:
├── settings
│ ├── elm.json
│ ├── elm-stuff
│ ├── src
│ └── tests
├── _share
│ ├── benchmarks
│ ├── elm.json
│ ├── elm-stuff
│ ├── src
│ └── tests
├── tv-study
│ ├── elm.json
│ ├── elm-stuff
│ ├── src
│ └── tests
and some of these elm.jsons depend on each other, so we have:
"source-directories": [
"../_factories",
"../_share/src",
"src"
],
It's because of this case that I'm replacing .. with __dotdot. If we wanted to blindly copy the paths otherwise, it would leak out of the .coverage/instrumented folder. This way it stays inside, like .coverage/instrumented/__dotdot/_factories/Foo.elm
As the TODO comment says, it doesn't solve this very comprehensively (but it works for my case, so I stopped. Sorry :) Again, I don't think this PR should be accepted, maybe think of it just as a "lessons learned" point for if you ever want to do this properly)
|
|
||
| elmPackage["name"] = "author/project"; | ||
| elmPackage["source-directories"] = elmPackage["source-directories"] | ||
| .map(sourceDir => fixOutOfRootPath(sourceDir)); | ||
| elmPackage["source-directories"].push("elm-coverage-src"); |
There was a problem hiding this comment.
This is a bit less likely to collide. (Instead of src/Coverage.elm the user would have to have a source directory elm-coverage-src and inside Coverage.elm.)
| @@ -227,7 +254,7 @@ var generateReport = (log, args) => () => { | |||
| return Promise.resolve(); | |||
| case "human": | |||
| log.info("generating", "Generating report..."); | |||
| return analyzer(args.path, data); | |||
| return analyzer(data); | |||
| module.exports = function(sourcePath) { | ||
| var revertOutOfRootFix = (path) => { | ||
| return path.replace(/__dotdot/g,'..'); | ||
| } |
There was a problem hiding this comment.
This is the part where we try to translate from __dotdot back to ... We can't do it too quick because of the path.split('.') inside the thing that later tries to translate Foo.Bar back to Foo/Bar.elm
| }); | ||
|
|
||
| return { | ||
| coverageData: coverageData, | ||
| moduleMap: moduleMap | ||
| moduleMap: moduleMap, | ||
| presentablePathMap: presentablePathMap, |
There was a problem hiding this comment.
And so we do the translation in a new dictionary and pass that on to the CLI table reporter and the Elm app.
|
I think this was closed automatically by GitHub due to master->main branch renaming, but still would benefit from being open (see some comments in issue #44) |
Connected to zwilias/elm-instrument#1
This is very very crude, solves the problem in a wrong way, creates more problems just to band-aid them later, but it unblocked me in my usecase.
The solved problem:
elm-coverageonly readssrc/directory for sourcesThe solution:
elm-coveragenow reads theelm.jsonfile for source directories and threads that filepath info throughout its executionIt would be best to ditch this particular solution (it's really a hot pile of mess) and instead refactor the code to not assume some things (single source directory), but I'm offering this in case it helps anybody :)