Skip to content
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

done ht-2 #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

davidoff0895
Copy link

No description provided.

PropTypes.shape({
rating: PropTypes.number.isRequired,
})
),
Copy link
Owner

Choose a reason for hiding this comment

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

тут shape и arrayOf тоже должны быть isRequired

PropTypes.shape({
id: PropTypes.string.isRequired,
name: PropTypes.string.isRequired,
})
Copy link
Owner

Choose a reason for hiding this comment

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

тут shape тоже должен быть isRequired

};

Review.propTypes = {
rating: PropTypes.number.isRequired,
Copy link
Owner

Choose a reason for hiding this comment

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

user и text все равно нужно описать, даже если они есть в defaultProps

reviews: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.string.isRequired,
})
Copy link
Owner

Choose a reason for hiding this comment

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

тут shape тоже должен быть isRequired

describe('Product', () => {
test('should contains Review component for each review', () => {
const wrapper = mount(<Reviews reviews={reviews} />);
expect(wrapper.find(Review).length).toBe(reviews.length);
Copy link
Owner

Choose a reason for hiding this comment

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

тут лучше проверять не наличие элементов, а сами имя юзера и прочее.


Enzyme.configure({ adapter: new Adapter() });

describe('useAmount', () => {
Copy link
Owner

Choose a reason for hiding this comment

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

это все лучше тестировать вместе с самим компонентом продукта, т.к. если перенести это состояние в компонент, то такие тесты нужно переписывать

beforeEach(() => jest.resetAllMocks());
test('should be initial amount when hook is called', () => {
jest
.spyOn(React, 'useState')
Copy link
Owner

Choose a reason for hiding this comment

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

не стоить мокать реакт

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants