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

Issue-#2923 added test for the Account Form. #2955

Merged
merged 5 commits into from
Aug 14, 2024

Conversation

gauravsingh94
Copy link
Contributor

@gauravsingh94 gauravsingh94 commented Jan 25, 2024

Ref #2923

Changes:
Added the test for AccountForm.

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@gauravsingh94
Copy link
Contributor Author

@lindapaiste

I have added the test for Account from please review it. if these test feel good to you then i started to write for the other component as well.

In the AccountForm test i have added the test for :

  • The form component renders correctly with the initial user values.
  • clicking on save all settings will call the dispatch the updateSettings.
  • when out data is submitting in this period our save all settings button must be disabled.

image

lindapaiste
lindapaiste previously approved these changes Jan 26, 2024
Copy link
Collaborator

@lindapaiste lindapaiste left a comment

Choose a reason for hiding this comment

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

These are very good tests! 👍

I'm often not a fan of mocking because it's testing a fake behavior and not the real behavior. But this is a case where it makes sense because I don't think that our dev/test setup has actual user accounts associated with it. Maybe it does? We should look into that. When it comes to testing the login screen it would be cool if we could try submitting a wrong password and verify that it fails vs a correct password and verify that it succeeds.

@gauravsingh94
Copy link
Contributor Author

@lindapaiste
Added the tests for the LoginForm.

@gauravsingh94
Copy link
Contributor Author

@lindapaiste
Added the test for the NewPasswordForm.

image

@gauravsingh94 gauravsingh94 force-pushed the issue-#2923 branch 3 times, most recently from 61577b9 to 1ca6b67 Compare February 9, 2024 12:49
@khanniie
Copy link
Collaborator

thanks, these are great tests! : ) lgtm to me overall but you will probably have to move the test locations as it looks like the files have moved locations a little bit

@raclim raclim merged commit 272645f into processing:develop Aug 14, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants