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

Added unit test to account-dialog.component #2254

Merged
merged 5 commits into from
Sep 2, 2023

Conversation

Tiago404
Copy link
Contributor

@Tiago404 Tiago404 commented Aug 19, 2023

Fixes

Part of #1717

Checks

  • Ran yarn test-build
  • Updated relevant documentations
  • Updated matching config options in altair-static

Changes proposed in this pull request:

This PR adds a test around the functionality of account-dialog.component:

  • handleLoginChange

@Tiago404
Copy link
Contributor Author

This is my first open source contribution, and I'm still pretty new to testing with Angular. This is a pretty simple first test I implemented, kind of just want to see how the process of getting a PR approved goes and see where I could improve.
I would like to implement more tests on this account-dialog component, but I am not sure exactly what else I could test.
I would appreciate any pointers.
Was thinking of implementing some tests for the openBillingPage() function, but am having a hard time figuring out how to implement tests for it.

Copy link
Collaborator

@imolorhe imolorhe left a comment

Choose a reason for hiding this comment

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

Hey, thanks for the changes!

Testing angular components (how the UI elements interact) isn't always easy/straighforward in my experience. However in Altair, we have a component testing library that builds on top of angular testing APIs and tries to make the testing process a bit easier. As much as possible, we should be using this API (in combination with jest assertions of course) instead of the default angular one.

Note: this library only applies when testing UI components. For all other things like business logic, services, pipes, etc. you should be able to test with simple jest

For testing the openBillingPage(), you'd want to jest.mock the apiClient and then check that the apiClient.getBillingUrl() has been called

Comment on lines 33 to 37
jest.spyOn(component.handleLoginChange, 'emit');

component.submitLogin();

expect(component.handleLoginChange.emit).toHaveBeenCalled();
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of using jest.spyOn, we could trigger the click event on the submit button (or submit event on the form) and check that the handleLoginChange event was emitted. For example,

wrapper.setProps({
collections: [
{
id: 1,
title: 'Collection 1',
queries: []
},
],
});
await wrapper.nextTick();
const sortOption = wrapper.find('nz-dropdown-menu li');
sortOption.emit('click');
expect(wrapper.emitted('sortCollectionsChange')).toBeTruthy();

@Tiago404 Tiago404 closed this Sep 2, 2023
@Tiago404 Tiago404 reopened this Sep 2, 2023
@Tiago404 Tiago404 requested a review from imolorhe September 2, 2023 07:20
Copy link
Collaborator

@imolorhe imolorhe left a comment

Choose a reason for hiding this comment

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

Looking good!

@imolorhe imolorhe added this pull request to the merge queue Sep 2, 2023
Merged via the queue into altair-graphql:master with commit 05f6af3 Sep 2, 2023
@Tiago404 Tiago404 deleted the unit-tests branch September 3, 2023 04:20
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