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

Code Coverage: Create tests for src/components/UserPortal/Login/Login.tsx #1032

Closed
palisadoes opened this issue Nov 5, 2023 · 28 comments
Closed
Labels
good first issue Good for newcomers test Testing application

Comments

@palisadoes
Copy link
Contributor

We will need unittests done for all methods, classes and functions found in this file.

Any widgets, components, modals referenced in this file must also have unittests done

PR Acceptance Criteria

@palisadoes palisadoes added the bug Something isn't working label Nov 5, 2023
@palisadoes palisadoes added good first issue Good for newcomers test Testing application and removed bug Something isn't working labels Nov 5, 2023
Copy link

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Nov 16, 2023
@MahendraDani
Copy link
Contributor

I read the current tests in Login.test.tsx and observed the current code coverage.
We can observe that codecov says only lines 54 and 55 are not covered in the tests :

} else {
toast.warn(t('notAuthorised'));

After trying all the possible case locally I found out that the control does not reach on the above lines in any cases. In other words when handleLogin function is triggered with non-empty email and password fields the data will always be returned and hence the else condition (when data is empty as returned by login mutation) never occurs.

Thus to get the test coverage of this component to 100% is to simply remove the else condition i.e

} else {
toast.warn(t('notAuthorised'));

It does not break the functioning of user portal and the test coverage also reaches to 100%.
Please let me know if this approach is correct or there is something else need to be done?
I would like to work on this issue

cc : @noman2002

@noman2002
Copy link
Member

Try to mock the situation in which it goes to the else condition. And write the test for the same. Removing the else condition will not be a good approach.

@MahendraDani
Copy link
Contributor

Try to mock the situation in which it goes to the else condition. And write the test for the same. Removing the else condition will not be a good approach.

I will try

@MahendraDani
Copy link
Contributor

@noman2002
Are admins and superadmin allowed to login into user portal?
Because when I was trying to mock all possible situations I found that we can login into user portal using admin or superadmin credentails.

Steps to reproduce

  1. Go to user portal - localhost:3000/user
  2. Enter credentials of admins or superadmins
  3. Click on Login button.
  4. You will be logged into user portal as admin or superadmin

@noman2002
Copy link
Member

Yes admin can login to user portal.

@MahendraDani
Copy link
Contributor

@noman2002
I am not able to mock the situation in which the else statement is executed.
Would you please help?

@MahendraDani
Copy link
Contributor

MahendraDani commented Dec 4, 2023

@noman2002
I have tried all possible mocks and I have represented the outputs in the form of a diagram.

Please observe from the diagram that there are only three possible cases when any person tries to login into user portal http://localhost:3000/user:

  • With no credentials i.e person clicks login button without entering email and password then they are responded with error toast message Please enter a valid password and email.
  • When person is either user,admin or superadmin they are logged into user portal succcessfully.
  • When person tries to login with credentials which are not in database then the response is a error toast with message User not found.

Note that all the above cases are already covered in the current tests.

We can observe that there is no other case than this in which control will execute line 54 and 55

What do you think? If there is anything that I missed please help me through.
Thanks

@MahendraDani
Copy link
Contributor

@noman2002 I have tried all possible mocks and I have represented the outputs in the form of a diagram.

Please observe from the diagram that there are only three possible cases when any person tries to login into user portal http://localhost:3000/user:

  • With no credentials i.e person clicks login button without entering email and password then they are responded with error toast message Please enter a valid password and email.
  • When person is either user,admin or superadmin they are logged into user portal succcessfully.
  • When person tries to login with credentials which are not in database then the response is a error toast with message User not found.

Note that all the above cases are already covered in the current tests.

We can observe that there is no other case than this in which control will execute line 54 and 55

What do you think? If there is anything that I missed please help me through. Thanks

@noman2002 Any suggestions?

Copy link

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Dec 16, 2023
@palisadoes
Copy link
Contributor Author

Unassigning due to inactivity and no related PR

@pateldivyesh1323
Copy link
Contributor

pateldivyesh1323 commented Dec 16, 2023

@noman2002, @palisadoes I would like to work on this.

There is no case which will enter into else condition as described by @MahendraDani. Actually that else condition was used in admin login where it used to filter normal users to not enter into admin portal. But in user portal admins can also login. In case if request does not carry any data it will always go to catch block.

@pateldivyesh1323
Copy link
Contributor

I am unassigning myself, this is now up for grabs.

@pateldivyesh1323 pateldivyesh1323 removed their assignment Dec 23, 2023
Copy link

github-actions bot commented Jan 4, 2024

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Jan 4, 2024
@shankeleven
Copy link

shankeleven commented Jan 5, 2024

please assign the issue to me, would love to work

@github-actions github-actions bot removed the no-issue-activity No issue activity label Jan 8, 2024
@Shashankpantiitbhilai
Copy link

I am interested,please assign this to me

@Cioppolo14
Copy link
Contributor

@shankeleven Sorry I missed this earlier! You currently have 2 issues in another repo, so I am going to assign it to @Shashankpantiitbhilai.

Copy link

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Jan 27, 2024
@Cioppolo14
Copy link
Contributor

Unassigning due to inactivity.

@Sauradip07
Copy link
Contributor

Please assign me I want to work on this issue

@Cioppolo14
Copy link
Contributor

@Sauradip07 Our policy is to assign no more than two issues to each contributor across all repositories. This way everyone gets a chance to participate in the projects. We sometimes give exceptions for more urgent cases and sometimes we lose track, but the policy stands. You have reached your limit, please wait until your existing issues are closed before requesting more issues. You could unassign yourself from one of the other issues too.

@gautam-divyanshu
Copy link
Member

Please assign me this issue , love to work on it.

@gautam-divyanshu
Copy link
Member

Please assign me this issue , love to work on it.
I am working on it.

@gautam-divyanshu gautam-divyanshu removed their assignment Feb 6, 2024
@karthxk07
Copy link
Contributor

@palisadoes Sir, Is this issue still open and stale? I can work on this.

@shankeleven
Copy link

shankeleven commented Feb 20, 2024

please assign this issue to me @palisadoes @Cioppolo14
would love to work on this

Copy link

github-actions bot commented Mar 4, 2024

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Mar 4, 2024
@Cioppolo14 Cioppolo14 removed the no-issue-activity No issue activity label Mar 4, 2024
@Cioppolo14
Copy link
Contributor

Unassigning due to no activity.

@pranshugupta54
Copy link
Member

Hey @Cioppolo14, @palisadoes. Is this completed?

Coverage looks 100%

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers test Testing application
Projects
None yet
Development

No branches or pull requests