Skip to content

test: categorynavbar component #952

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 18, 2022
Merged

test: categorynavbar component #952

merged 1 commit into from
May 18, 2022

Conversation

@Diegoalbag
Copy link
Contributor

@Diegoalbag Diegoalbag commented May 5, 2022

Description

Added unit tests for packages/theme/modules/catalog/category/components/navbar/CategoryNavbar.vue

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Screen Shot 2022-05-05 at 3 25 27 PM

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

bartoszherba
bartoszherba requested changes May 9, 2022
View reviewed changes
Copy link
Collaborator

@bartoszherba bartoszherba left a comment

Choose a reason for hiding this comment

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

@Diegoalbag Please, fix also the PR name so it align with the rules (remove the dot ;) )

packages/theme/modules/catalog/category/components/navbar/__tests__/CategoryNavbar.spec.js Outdated
expect(uiStateMock.toggleFilterSidebar).toHaveBeenCalledTimes(1);
});

it('Should render "sort by" select and change selection', async() => {
Copy link
Collaborator

@bartoszherba bartoszherba May 9, 2022

Choose a reason for hiding this comment

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

Can we also verify that select has all expected options based on its data?

Copy link
Collaborator

@bartoszherba bartoszherba May 17, 2022

Choose a reason for hiding this comment

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

@Diegoalbag What about this one?

packages/theme/modules/catalog/category/components/navbar/__tests__/CategoryNavbar.spec.js Outdated

expect(uiStateMock.changeToCategoryGridView).toHaveBeenCalledTimes(1);
});
}); No newline at end of file
Copy link
Collaborator

@bartoszherba bartoszherba May 9, 2022

Choose a reason for hiding this comment

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

I think that we can also add the following tests:

  1. Check if the items count is rendered properly with the expected value
  2. Check if the component is in loading state all skeletons are rendered as expected
  3. change sorting action emits 'reloadProducts' event

Copy link
Collaborator

@bartoszherba bartoszherba May 17, 2022

Choose a reason for hiding this comment

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

@Diegoalbag What about this one?

packages/theme/modules/catalog/category/components/navbar/__tests__/CategoryNavbar.spec.js Outdated
import { render } from '~/test-utils';
import CategoryNavbar from '../CategoryNavbar.vue';
import { paginationData, sortByData} from '~/test-utils/mocks/categoryNavbarMock';
import { expect } from '@jest/globals';
Copy link
Collaborator

@bartoszherba bartoszherba May 9, 2022

Choose a reason for hiding this comment

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

Screenshot 2022-05-09 at 09 08 14
It is not required to import assertion anyway.

Copy link
Collaborator

@bartoszherba bartoszherba May 17, 2022

Choose a reason for hiding this comment

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

@Diegoalbag What about this one?

Copy link
Contributor Author

@Diegoalbag Diegoalbag May 17, 2022

Choose a reason for hiding this comment

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

@bartoszherba Assertion is not being imported anymore.

@Diegoalbag Diegoalbag force-pushed the test/m2-4339-add-test-category-navbar-component branch from 7a24ac9 to 8a4312c Compare May 16, 2022 14:42
@Diegoalbag Diegoalbag changed the title test: CategoryNavbar.vue component test: categorynavbar component May 16, 2022
@Diegoalbag Diegoalbag force-pushed the test/m2-4339-add-test-category-navbar-component branch from 8a4312c to 10315fe Compare May 16, 2022 14:44
@Diegoalbag Diegoalbag requested a review from bartoszherba May 16, 2022 14:45
@sethidden
Copy link
Contributor

sethidden commented May 17, 2022

I recently added a GitHub action step that runs ESLint and fails if there are any ESLint errors.
If the build action starts failing in this PR, please:

  1. Rebase the branch from this PR on develop then push out the changes.
  2. If that doesn't work, this means your new code has some linting errors. Please go to the repository root (where the .git folder is) and run yarn lint

Diegoalbag
Diegoalbag commented May 17, 2022
View reviewed changes
packages/theme/modules/catalog/category/components/navbar/__tests__/CategoryNavbar.spec.js Outdated
const { getByRole, getByText, getAllByRole, emitted } = render(CategoryNavbar, categoryNavbarProps);
const sortSelect = getByRole('combobox');
const options = getAllByRole('option');
expect(options.length).toBe(categoryNavbarProps.props.sortBy.options.length);
Copy link
Contributor Author

@Diegoalbag Diegoalbag May 17, 2022

Choose a reason for hiding this comment

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

@bartoszherba here I verify that select has all expected options based on its data

Diegoalbag
Diegoalbag commented May 17, 2022
View reviewed changes
packages/theme/modules/catalog/category/components/navbar/__tests__/CategoryNavbar.spec.js

await userEvent.selectOptions(sortSelect, ['name_ASC']);
expect(getByText('Sort: Name A-Z').value).toBe(sortSelect.value);
expect(emitted().reloadProducts).toHaveProperty('length', 1);
Copy link
Contributor Author

@Diegoalbag Diegoalbag May 17, 2022

Choose a reason for hiding this comment

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

@bartoszherba Here I check that changing sorting action emits 'reloadProducts' event

@Diegoalbag Diegoalbag force-pushed the test/m2-4339-add-test-category-navbar-component branch from 10315fe to 3e617f8 Compare May 17, 2022 15:31
Diegoalbag
Diegoalbag commented May 17, 2022
View reviewed changes
packages/theme/modules/catalog/category/components/navbar/__tests__/CategoryNavbar.spec.js
const loadingCategoryNavbarProps = { props: { pagination: paginationData, sortBy: sortByData, isLoading: true } };

describe('CategoryNavbar.vue', () => {
it('Should render Skeleton when loading is true', () => {
Copy link
Contributor Author

@Diegoalbag Diegoalbag May 17, 2022

Choose a reason for hiding this comment

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

@bartoszherba Check if the component is in loading state all skeletons are rendered as expected

@Diegoalbag Diegoalbag force-pushed the test/m2-4339-add-test-category-navbar-component branch 3 times, most recently from f292994 to 350baaa Compare May 17, 2022 18:53
@Diegoalbag Diegoalbag force-pushed the test/m2-4339-add-test-category-navbar-component branch from 350baaa to 42107a3 Compare May 17, 2022 19:05
@bartoszherba bartoszherba merged commit 5c95740 into vuestorefront:develop May 18, 2022
@Frodigo Frodigo added this to the 1.0.0-rc.8 milestone May 23, 2022
@Frodigo Frodigo mentioned this pull request May 26, 2022
Merged
@Frodigo Frodigo mentioned this pull request Jun 2, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bartoszherba bartoszherba bartoszherba approved these changes

@Frodigo Frodigo Awaiting requested review from Frodigo

@sethidden sethidden Awaiting requested review from sethidden

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.0.0-rc.8

Development

Successfully merging this pull request may close these issues.

4 participants

@Diegoalbag @sethidden @bartoszherba @Frodigo