Repaired require for default .gemini.js and .gemini.json

[twolfson]

I'm experimenting with a script that requires a dynamically computed .gemini.js (depends on node module for path to executable). However, when I ran Gemini I saw that it was getting a MODULE_NOT_FOUND error for .gemini.js despite it working earlier for .gemini.yml.

After some digging, I found that we were essentially running require('.gemini.js') which tells Node.js to look for a package named .gemini.js instead of the local file (i.e. ./.gemini.js). I have resolved that in this PR by moving to an absolute path for .gemini.js/.gemini.json.

In this PR:

• Updated getDefaultConfig to return absolute path for non-YAML files

Notes:

We could use fs.readFile and JSON.parse/vm.runInContext (since fs.readFile resolves from process.cwd() but this is much simpler.

[twolfson]

Gah, I forgot to point to stable

[twolfson]

Oh, never mind. There's a stable documented in CONTRIBUTING.md but it doesn't exist =/

https://github.com/gemini-testing/gemini/blob/v3.0.2/CONTRIBUTING.md

[coveralls]

Coverage decreased (-1.0%) to 75.572% when pulling 6a34899 on twolfson:dev/fix.gemini.js.require.sqwished into ed2fae0 on gemini-testing:master.

[coveralls]

Coverage decreased (-1.0%) to 75.572% when pulling 6a34899 on twolfson:dev/fix.gemini.js.require.sqwished into ed2fae0 on gemini-testing:master.

[j0tunn]

I think we can do it for yml file too

[twolfson]

Sure thing. I initially left yml out to prevent long paths in case there was an error later and it popped up in the stack trace.

[twolfson]

Alright, I have addressed the comments and updated the PR:

• Added YML absolute path resolution
• Added tests for resolving a default config

[coveralls]

Coverage decreased (-0.6%) to 77.489% when pulling 8d9e3c9 on twolfson:dev/fix.gemini.js.require.sqwished into f28ea8e on gemini-testing:master.

[j0tunn]

@twolfson Thanks, but I was thinking about some unit-tests may be. And thats not a good idea to make getDefaultConfig public only for test purposes.
I'll take a look in a few days

[twolfson]

Ah, alright. I think using unit tests will be too overwhelming for me.

With respect to the current status, it's possible to have the same result in functional without getDefaultConfig but we would have to set a distinct variable. For example:

// In `default-js/.gemini.js`
projectRoot: '/it/works/default-js'

// In our tests
process.chdir('default-js');
var config = new Config();
assert.deepPropertyVal(config, 'system.projectRoot', '/it/works/default-js');

However, I will leave the PR as is until your exploration with unit tests.

[j0tunn]

Closing in favor of #442
@twolfson Sorry for long delay

[twolfson]

Glad to hear it's been patched. Fwiw, #442 doesn't seem to have any tests to prevent this regression from recurring (i.e. nothing verifying default path is absolute) =/

[j0tunn]

Fwiw, #442 doesn't seem to have any tests to prevent this regression from recurring (i.e. nothing verifying default path is absolute) =/

https://github.com/gemini-testing/gemini/pull/442/files#diff-7cd03a08e3e1393e232f4490c33a335aR22

[twolfson]

That line is a mock setup in the test, it doesn't verify that the actual config-reader code uses path.resolve =/

https://github.com/gemini-testing/gemini/pull/442/files#diff-435eaee6ef8a8a771ac5e6ca51dac9d6R26

[j0tunn]

Those tests will fail without that line. Take a look at this commit: 88de4a9

[twolfson]

After playing around with it myself, I realized what is going on:

• fs.readdirSync and proxyquire are emulating the filesystem and only defining absolute paths
• getDefaultConfig must return an absolute path, otherwise it won't match the defined absolute path

As a heads up, while this works for now it will cause a maintenance tax any time the implementation is changed (e.g. moving to JSON.parse for JSON files will require a restructuring of the tests).

I should also note that that PR doesn't test .gemini.yml loading =/

[twolfson]

And there's also no test for what happens when no config is found =/

[j0tunn]

Yep, we doesn't have 100% coverage. And we write tests when we see bug or on some code modification.
You can send PR if you wish.