Skip to content
This repository has been archived by the owner on Sep 21, 2022. It is now read-only.

fix incorrect JS config resolving #705

Merged
merged 1 commit into from
Jun 23, 2017
Merged

fix incorrect JS config resolving #705

merged 1 commit into from
Jun 23, 2017

Conversation

mad-gooze
Copy link

fixes #704

@tormozz48
Copy link
Contributor

/cc @sipayRT

umbobabo pushed a commit to umbobabo/gemini that referenced this pull request Jan 16, 2017
@DudaGod
Copy link
Member

DudaGod commented Jan 24, 2017

Can you write tests on it? And rename please commit by these rules - https://github.com/conventional-changelog/standard-version#commit-message-convention-at-a-glance

@mad-gooze
Copy link
Author

done

@@ -28,7 +28,7 @@ function getDefaultConfig() {

function requireModule(file) {
try {
return require(file);
return require(path.resolve(file));
Copy link
Contributor

Choose a reason for hiding this comment

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

в getDefaultConfig можно тогда убрать path.resolve

const testData = {some: 'data'};
const files = {};
files[fileName] = testData;
const reader = initReader_(files);
Copy link
Contributor

Choose a reason for hiding this comment

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

Вот это можно схлопнуть в одну строчку:

const test_ = (fileName) => {
    it('should read ' + fileName, () => {
        const reader = initReader_({[fileName]: {foo: 'bar'}});

        const result = reader.read();

        assert.deepEqual(result, {foo: 'bar'});
    });
};

И тогда для твоих тестов ты эту функцию можешь сдублировать вместо того, чтобы добавлять в нее логику. Сейчас тест читается трудновато

@sipayRT
Copy link
Member

sipayRT commented Jun 23, 2017

ping

@mad-gooze
Copy link
Author

Внес правки

@j0tunn j0tunn merged commit d63bb78 into gemini-testing:master Jun 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom JS config file provided from CLI option does not resolve correctly
6 participants