Skip to content

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

@raclim raclim added the Area: Testing For tests, test coverage, or testing infrastructure label Jan 25, 2024
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
Member

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Testing For tests, test coverage, or testing infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants