Ignore files that did not match specified mask. #9
Conversation
| const getFilesByMask = (pattern, options) => { | ||
| return q.nfcall(glob, pattern, options).then((paths) => { | ||
| return _.isEmpty(paths) && !isMask(pattern) | ||
| ? q.reject(new Error(`Cannot find files by mask ${pattern}`)) |
There was a problem hiding this comment.
тут уже message ошибки должен быть другой.
сейчас он не соответствует действительности, так как дословно такой Не могу найти файлы по маске, а мы как раз таки в этом пулл-реквесте реализуем логику, чтобы не ругаться на маски.
There was a problem hiding this comment.
типо надо писать что-то вроде: Не могу найти файлы по пути 'какой-то плохой путь'
| const isMask = (pattern) => glob.hasMagic(pattern); | ||
|
|
||
| const getFilesByMask = (pattern, options) => { | ||
| return q.nfcall(glob, pattern, options).then((paths) => { |
| exports.isMasks = (paths) => { | ||
| return [].concat(paths).every((path) => glob.hasMagic(path)); | ||
| }; | ||
| exports.isMasks = (patterns) => [].concat(patterns).every(isMask); |
There was a problem hiding this comment.
а ты эту штуку, где еще используешь?
ты типо добавил новый метод в модуль globExtra ?
Если да, то я бы отсюда экспортил функцию isMask, а не функцию isMasks (кстати isMasks так себе название, кажется, с точки зрения английского языка)
There was a problem hiding this comment.
@eGavr так ты разве не видишь, что этот метод там уже был, до моих вмешательств, и все что я сделал, так это сделал из ф-ии однострочник и завернул glob.hasMagis в ф-ию хелпер, потому что она у меня используется уже в 2-х местах. Нейминг и вправду не очень, но к сожалению у меня не достаточно ресурсов, чтобы выяснить где оно может быть использовано.
There was a problem hiding this comment.
ну типо давай узнаем, кто ее написал, зачем она нужна ? :)
сс @rostik404
There was a problem hiding this comment.
Судя по blame, это Дима добавил
cc @DudaGod
There was a problem hiding this comment.
Да, это я добавлял. Использовал ее чтобы ускорить запуск тестов в gemini в случае если все сеты указаны масками. Если все сеты - маски, то мне не нужно каждую из них раскрывать (на это уходит дохрена времени, перед запуском 2-4 секунды), тем более если пользователь хочет запустить всего один тест (указанный в командной строке). В этом случае я матчу файл на маски с помощью micromatch-а и определяю запускаем тест или нет.
There was a problem hiding this comment.
экспортить надо просто isMask
Кажется, что писать в коде isMask(path) или paths.every(isMask) не сильно сложно
Нейминг и вправду не очень, но к сожалению у меня не достаточно ресурсов, чтобы выяснить где оно может быть использовано
Поиск по гитхабу дает всего 18 результатов, не так много людей пишут такую фигню с точки зрения английского языка :)
| @@ -38,10 +38,28 @@ describe('path-utils', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| it('should throw an error if a mask does not match files', () => { | |||
| it('should throw error if mask contains unexistent file path', () => { | |||
There was a problem hiding this comment.
вообще я бы назвал эту сущность выражением или паттерном, а то получается тафтология - путь содержит несуществующий путь.
| return assert.isRejected(globExtra.expandPaths(['bad/mask/file.js']), 'Cannot find files by mask bad/mask/file.js'); | ||
| }); | ||
|
|
||
| it('should throw error if mask contains unexistent directory path', () => { |
| return assert.isRejected(globExtra.expandPaths(['bad/mask']), 'Cannot find files by mask bad/mask'); | ||
| }); | ||
|
|
||
| it('should ignore masks that contain magic symbols', () => { |
There was a problem hiding this comment.
that –> which
that используется для одушевленных предметов
| 'bad/mask/*.js', | ||
| 'some/path/*.js' | ||
| ]).then((absolutePaths) => { | ||
| assert.deepEqual(absolutePaths, ['some/path/file.js']); |
|
Если ты еще добавляешь в public API новый метод, то на него отдельно тоже стоит тесты написать. |
|
@eGavr Мной не было добавлено ничего в public API, а на isMasks тесты уже есть - https://github.com/gemini-testing/glob-extra/blob/master/test/index.test.js#L221 |
|
хмммм, а где оно используется тогда? cc @rostik404 |
|
@eGavr в gemini |
| return assert.isRejected(globExtra.expandPaths(['bad/mask']), 'Cannot find files by mask bad/mask'); | ||
| }); | ||
|
|
||
| it('should ignore masks which contain magic symbols', () => { |
There was a problem hiding this comment.
здесь проверяется, что игнорятся маски, которые не матчатся на файлы, а в тайтле написано другое
|
/ok |
Currently, if glob call did not find any file for one of passed to expandPaths masks all execution will be failed. I think it should just ignore this behavior or just warn about this.