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

Additional login page test #91

Merged
merged 14 commits into from
Apr 16, 2024
Merged

Additional login page test #91

merged 14 commits into from
Apr 16, 2024

Conversation

FernFinder
Copy link
Contributor

@FernFinder FernFinder commented Apr 15, 2024

Overview

Added additional tests to the login page. The last thing big test that needs to be added is the function test. This will need #81 to be resolved first.

Changes

Test 'renders without crashing' is more thorough now.

Added

Test 'input field updates value with current input' added to test input fields and see if they are updating values appropriately.

Changed

General comments added for clarity.

Instructions for Testing

See GitHub Actions tab.

Checklist before Merging

  • I have performed a self-review of my own code.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if appropriate).
  • I have ensured the changes adhere to the project's coding standards and guidelines.

Additional Comments

Assistance from ChatGPT 3.5 used. Appropriate annotations used.
Conversation here: https://chat.openai.com/share/025568dc-22e7-4567-8d68-6e1a6b5b9cf8.
Slight modifications for my user case and testing of both input fields.

Copy link
Contributor

@vicliyj vicliyj left a comment

Choose a reason for hiding this comment

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

Seems like there is something failing in the workflow!

@FernFinder FernFinder closed this Apr 15, 2024
@FernFinder FernFinder reopened this Apr 15, 2024
@FernFinder
Copy link
Contributor Author

Sorry about that, I was testing something else but I forgot to use a new branch. I reverted the commits so now the tests will pass.

Copy link
Contributor

@vicliyj vicliyj left a comment

Choose a reason for hiding this comment

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

Pull Request Review:

  • Readability: Easy to follow and understand!
  • Quality: Looks good!
  • Functionality: Added unit test for login page.
  • Housekeeping: AI use noted down where it was used, comments in place for what code does.

Great work!

Copy link
Contributor

@DillonDavidson DillonDavidson left a comment

Choose a reason for hiding this comment

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

Pull Request Review:

Readability: Code is easy to read and understand
Quality: Quality of pull request is good, nice job
Functionality: Included a test for the added changes
Housekeeping: Comments added designating where AI was used for code generation.

Good job Fernando!

@FernFinder FernFinder merged commit 74ac243 into UNLV-CS472-672:main Apr 16, 2024
1 check passed
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.

3 participants