always use an empty pelias.json in test #1431
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
orangejulius
reviewed
May 7, 2020
package.json
Outdated
| @@ -15,7 +15,7 @@ | |||
| "start": "./bin/start", | |||
| "test": "npm run unit", | |||
| "travis": "npm test", | |||
| "unit": "./bin/units", | |||
| "unit": "PELIAS_CONFIG=./test/pelias-config-empty.json ./bin/units", | |||
There was a problem hiding this comment.
This actually would work great in ./bin/units itself (I think). That way anyone who runs the unit tests through any mechanism would get the right PELIAS_CONFIG
|
Ah yeah, we should have done this a long time ago. It looks like we need the actual empty |
|
I went in and fixed it all myself, and will take care of merging this from here. Thanks for not putting up with the cruft we've let sit for too long :P |
orangejulius
added a commit
that referenced
this pull request
Apr 1, 2021
This change resolves #1419 by ensuring all unit tests will run with the `api.targets.auto_discover` property set to false, even if the value of that property in the `pelias-config` defaults or `~/pelias.json` is set to true. There were two changes required to achieve this: First, expand upon the work in #1431 that added an empty `pelias.json` file for use in the unit tests. Its now no longer empty and includes setting `auto_discover` to false. Second, one unit test still ended up calling Elasticsearch with that change. It was a test for the `TypeMapping` module itself, and it was overriding `pelias-config` with its own. Now with a test-specific `pelias.json` file, that code can simply be removed, and the unit tests can use the unmodified `pelias-config` library. I tested these changes against pelias/config#127 to make sure that PR won't break anything, and I believe everything is good to go.
orangejulius
added a commit
that referenced
this pull request
Apr 8, 2021
This change resolves #1419 by ensuring all unit tests will run with the `api.targets.auto_discover` property set to false, even if the value of that property in the `pelias-config` defaults or `~/pelias.json` is set to true. There were two changes required to achieve this: First, expand upon the work in #1431 that added an empty `pelias.json` file for use in the unit tests. Its now no longer empty and includes setting `auto_discover` to false. Second, one unit test still ended up calling Elasticsearch with that change. It was a test for the `TypeMapping` module itself, and it was overriding `pelias-config` with its own. Now with a test-specific `pelias.json` file, that code can simply be removed, and the unit tests can use the unmodified `pelias-config` library. I tested these changes against pelias/config#127 to make sure that PR won't break anything, and I believe everything is good to go. Closes #1419
orangejulius
added a commit
that referenced
this pull request
Apr 9, 2021
This change resolves #1419 by ensuring all unit tests will run with the `api.targets.auto_discover` property set to false, even if the value of that property in the `pelias-config` defaults or `~/pelias.json` is set to true. There were two changes required to achieve this: First, expand upon the work in #1431 that added an empty `pelias.json` file for use in the unit tests. Its now no longer empty and includes setting `auto_discover` to false. Second, one unit test still ended up calling Elasticsearch with that change. It was a test for the `TypeMapping` module itself, and it was overriding `pelias-config` with its own. Now with a test-specific `pelias.json` file, that code can simply be removed, and the unit tests can use the unmodified `pelias-config` library. I tested these changes against pelias/config#127 to make sure that PR won't break anything, and I believe everything is good to go. Closes #1419
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed that if I had a pelias.json in api/ or in ~/, it seemed like it would get used by tests and a couple of them would break because they were taking in some of my pelias configuration (other sources mostly).
To make unit tests always pass for developers, always point the unit test command at an empty pelias.json in test/
Closes #1239