Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read source-dirs from elm.json #109

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
# Changelog

All notable changes to this project will be documented in this file.

The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
## 6.0.0

- Change elm-verify-examples.json to contain elm root and not source root. In addition it will respect source-directories defined in elm.json [#109](https://github.com/stoeffel/elm-verify-examples/pull/109)
stoeffel marked this conversation as resolved.
Show resolved Hide resolved

## 5.3.0

- Do not run tests by default, only generate [#65](https://github.com/stoeffel/elm-verify-examples/issues/65)
- Add --run-tests (-r) option to run generated tests [#65](https://github.com/stoeffel/elm-verify-examples/issues/65)


## 2.0.0

- We have a changelog now :tada:
Expand Down
5 changes: 3 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.PHONY: npm-install build test watch release-major release-minor release-patch
.PHONY: npm-install build test watch release-major release-minor release-patch run-examples verify-own-docs

npm-install:
npm install
Expand All @@ -11,8 +11,9 @@ verify-own-docs: build

test: verify-own-docs
./run-tests.sh

run-examples: build
cd example && ../bin/cli.js
cd example && ../bin/cli.js --run-tests

watch:
watchexec \
Expand Down
2 changes: 1 addition & 1 deletion Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ $ touch tests/elm-verify-examples.json

```json
{
"root": "../src",
"elm-root": "..",
"tests": ["Mock", "Mock.Foo.Bar.Moo", "./README.md"]
}
```
Expand Down
19 changes: 18 additions & 1 deletion bin/cli-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,26 @@ function loadVerifyExamplesConfig(configPath) {
*/

var verifyExamples = null;
var elmJson = null;

try {
verifyExamples = require(configPath);
if (verifyExamples["elm-root"] === undefined) {
throw new Error(
`elm-verify-examples.json at ${configPath} does not have an elm-root key`
knuton marked this conversation as resolved.
Show resolved Hide resolved
);
}
if (verifyExamples["root"] !== undefined) {
throw new Error(
`elm-verify-examples.json at ${configPath} has a root key, but it should be elm-root and point to the location of elm.json`
stoeffel marked this conversation as resolved.
Show resolved Hide resolved
);
}
var elmJsonPath = path.join(
path.dirname(configPath),
verifyExamples["elm-root"],
"elm.json"
);
elmJson = require(elmJsonPath);
} catch (e) {
console.log(`Copying initial elm-verify-examples.json to ${configPath}`);
fsExtra.copySync(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So perhaps here we can now just remove this bit, and instead just

var elmJsonPath = path.join(
       path.dirname(configPath),
       "..",
       "elm.json"
     );
     elmJson = require(elmJsonPath);
   verifyExamples = {};
}

or some such

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I thought we just make it optional not remove it completely, but maybe removing it is fine as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By optional I mean that the file doesn't have to exist - if it doesn't you get sensible defaults that should work for most projects.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay removed completely.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're slightly talk past each other. What I've been proposing is:

Make the existence of elm-verify-examples.json file optional. If it is missing, the default place we find elmRoot is in the containing folder. The default for which modules to run is the exposed ones plus Readme.md.

This change instead removes the ability to configure the elmRoot. I think that's also interesting, although probably to maintain some flexibility, I'd suggest searching containing folders recursively.

Expand All @@ -23,7 +40,7 @@ function loadVerifyExamplesConfig(configPath) {
));
}

return resolveTests(configPath, verifyExamples);
return resolveTests(configPath, Object.assign({}, verifyExamples, elmJson));
}

function resolveTests(configPath, config) {
Expand Down
35 changes: 26 additions & 9 deletions bin/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ function generate(model, allTestsGenerated) {
});
});
} else {
var pathToModule = path.join(
model.testsPath,
model.root,
var pathToModule = findModule(
path.join(model.testsPath, model["elm-root"]),
model["source-directories"],
elmModuleToPath(inputName)
);

Expand Down Expand Up @@ -154,7 +154,13 @@ function forFiles(model, files) {
.filter(function (v) {
return v.endsWith(".elm");
})
.map(elmPathToModule(model.root, model.testsPath));
.map(
elmPathToModule(
model["elm-root"],
model["source-directories"],
model.testsPath
)
);

return model;
}
Expand Down Expand Up @@ -215,19 +221,30 @@ function writeFile(testsDocPath) {
};
}

function elmPathToModule(root, testsPath) {
function elmPathToModule(elmRoot, sourceDirs, testsPath) {
return function (pathName) {
var relativePath = path.relative(
path.resolve(path.join(testsPath, root)),
pathName
);
var relativePath = findModule(elmRoot, sourceDirs, pathName);
if (relativePath.startsWith("./")) {
relativePath = relativePath.substr(2);
}
return relativePath.substr(0, relativePath.length - 4).replace(/\//g, ".");
};
}

function findModule(elmRoot, sourceDirs, pathName) {
gampleman marked this conversation as resolved.
Show resolved Hide resolved
for (var i = 0; i < sourceDirs.length; i++) {
const dir = sourceDirs[i];
const module = path.join(elmRoot, dir, pathName);
if (fs.existsSync(module)) {
return module;
}
Comment on lines +232 to +234
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optionally: Require exactly one match, failing explicitly if module reference is ambiguous (> 1 matches).

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think elm make fails if there are more than one modules with the same name.

}
console.error(
`Could not find module ${pathName} in ${sourceDirs} with root ${elmRoot}`
);
process.exit(1);
}

function elmModuleToPath(moduleName) {
return moduleName.replace(/\./g, "/") + ".elm";
}
Expand Down
2 changes: 1 addition & 1 deletion bin/templates/elm-verify-examples.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"root": "../src",
"elm-root": "..",
"tests": []
}
50 changes: 24 additions & 26 deletions example/elm.json
Original file line number Diff line number Diff line change
@@ -1,30 +1,28 @@
{
"type": "application",
"source-directories": [
"src"
],
"elm-version": "0.19.0",
"dependencies": {
"direct": {
"elm/browser": "1.0.0",
"elm/core": "1.0.0",
"elm/html": "1.0.0",
"elm/json": "1.0.0",
"elm/regex": "1.0.0",
"elm-community/list-extra": "8.0.0"
},
"indirect": {
"elm/time": "1.0.0",
"elm/url": "1.0.0",
"elm/virtual-dom": "1.0.0"
}
"type": "application",
"source-directories": ["src", "src-other/util"],
"elm-version": "0.19.0",
"dependencies": {
"direct": {
"elm/browser": "1.0.0",
"elm/core": "1.0.0",
"elm/html": "1.0.0",
"elm/json": "1.0.0",
"elm/regex": "1.0.0",
"elm-community/list-extra": "8.0.0"
},
"test-dependencies": {
"direct": {
"elm-explorations/test": "1.0.0"
},
"indirect": {
"elm/random": "1.0.0"
}
"indirect": {
"elm/time": "1.0.0",
"elm/url": "1.0.0",
"elm/virtual-dom": "1.0.0"
}
},
"test-dependencies": {
"direct": {
"elm-explorations/test": "1.0.0"
},
"indirect": {
"elm/random": "1.0.0"
}
}
}
13 changes: 13 additions & 0 deletions example/src-other/util/Other.elm
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module Other exposing (other)

{-| -}


{-| Stuff
other 4 --> 4
-}
other : Int -> Int
other x =
x + 1
13 changes: 5 additions & 8 deletions example/tests/elm-verify-examples.json
Original file line number Diff line number Diff line change
@@ -1,26 +1,23 @@
{
"root": "../src",
"elm-root": "..",
"tests": [
"Mock",
"Mock.Foo.Bar.Moo",
"Robustness",
"Failing",
"Todo",
"FalseTodo",
"README.md"
"README.md",
"Other"
],
"ignoreWarnings": {
"Mock.Foo.Bar.Moo": [
{
"name": "dontCare",
"ignore": [
"NoExampleForExposedDefinition"
]
"ignore": ["NoExampleForExposedDefinition"]
},
{
"ignore": [
"ExposingDotDot"
]
"ignore": ["ExposingDotDot"]
}
]
}
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion run-tests.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env bash

PASSED_COUNT=34
FAILED_COUNT=2
FAILED_COUNT=3
TODO_COUNT=1

pushd example
Expand Down
6 changes: 2 additions & 4 deletions tests/elm-verify-examples.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
{
"root": "../src",
"tests": [
"String.Util"
]
"elm-root": "..",
"tests": ["String.Util"]
}