-
Notifications
You must be signed in to change notification settings - Fork 2
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
CI preparations #14
CI preparations #14
Conversation
|
@zxqfox @Yeti-or and @awinogradov can some of you check? |
I'll wait till sunday and merge. Please check if you can |
() => (cell, naming) => { | ||
expect(naming).toBe('[origin]'); | ||
|
||
return `${cell.layer}.blocks/${cell.entity.block}/${cell.entity.block}.${cell.tech}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file.naming.stringify
doesnt work well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also mocked getter of naming (see few lines before - naming is simple string) so true bem/sdk.naming.*.stringify
will fail.
Plugin doesn't use bem/sdk.*
directly only through "helpers". After #10 we will remove helpers and such mocks won't be needed.
On the other side mocking of all dependencies is good for unit testing. Whole thing is tested by e2e
test/unit/generator.test.js
Outdated
it('should filter nonexistent files', () => { | ||
fs.existsSync.mockImplementation(path => { | ||
switch (path) { | ||
case 'dir/foo.js': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не хочешь оформить в виде hash?
const inMemoryFs = { dir: { 'foo.js': 'qwe' } }
, и возвращать _.flat(inMemoryFs).hasOwnKey(path)
Ну или хотя бы return {'dir/foo.js': true, 'dir/foo.js': false}[path];
Switch выглядит громостко
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like second:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now fs.existsSync.mockImplementation(path => path === 'dir/foo.js');
is the smallest
Addressed to #11 and #3