-
Notifications
You must be signed in to change notification settings - Fork 6
Feat #18 - wait parameter is boosted - code coverage #23
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
Conversation
src/index.jsx
Outdated
return Boolean(wait) | ||
} | ||
|
||
isLoadAFunction = () => { |
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.
Delete this ?
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.
done
src/index.spec.js
Outdated
it('should call the `load` function parameter if present', () => { | ||
// Mount | ||
const load = spy(() => {}) | ||
const load2 = spy(() => {}) |
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.
Maybe a more explicit naming ?
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.
done
@Zenika/nantes review plz plz |
// Implicitly meaning that this is an array of props | ||
if (Array.isArray(wait)) { | ||
return wait | ||
.map(w => Boolean(this.props[w])) |
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.
Use better naming convention for variables
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.
done
src/index.spec.js
Outdated
// Load function is not called twice | ||
// Graphic component is called | ||
// Loader shouldn't be printed | ||
load.should.have.been.called.once() |
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.
You don't have to call twice the line called.once(). Parenthesis are unnecessary
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.
done
src/index.spec.js
Outdated
// Load function is called | ||
// Graphic component isn't printed | ||
// Loader should be Dots | ||
load.should.have.been.called.once() |
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.
The load.should.have.been.called.once() is unnecessary because load.should.have.been.called.twice() already test this.
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 let this that way
src/index.spec.js
Outdated
// Load function is not called twice | ||
// Graphic component is called | ||
// Loader shouldn't be printed | ||
load.should.have.been.called.once |
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.
You could use a function to avoid repetition
src/index.spec.js
Outdated
expect(loaded.find(Dots).node).to.be.undefined | ||
} | ||
|
||
const isLoaded = (load, loaded, CustomLoader = undefined) => { |
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.
Delete = undefined
or = Dots
src/index.spec.js
Outdated
// Change `loaded` value | ||
loaded.setProps({ loaded: true }) | ||
|
||
// Load function is not called twice |
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.
Move commentaries with functions
No description provided.