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 tests for contexts/FontSizeContext and pages/SettingsPage #100

Merged

Conversation

DillonDavidson
Copy link
Contributor

@DillonDavidson DillonDavidson commented Apr 16, 2024

Overview

I was looking through the test files and saw that I never made one for FontSizeContext.js. My bad team! I also got full coverage for the SettingsPage.js as well.

Related Issue(s)

Adds more coverage

Changes

Created a FontSizeContext test file. Updated and finalized the SettingsPage test file

Added

  • tests/FontSizeContext.test.js

Changed

  • tests/SettingsPage.test.js

Removed

  • N/A

Fixed

  • N/A, just increasing coverage

Screenshots

image

Instructions for Testing

  • npm install
  • npm run coverage

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

One of these files was AI generated with the help of ChatGPT (FontSizeContext.test.js) and the other was NOT AI generated.

@dusek2
Copy link
Contributor

dusek2 commented Apr 16, 2024

Pull Request Review:
Readability:
Code is clean and well-documented.

Code Quality:
High-quality, maintainable code.

Testing:
Allmost Full coverage achieved.

Functionality:
Functions as expected.

Housekeeping:
PR is clean and focused.

Additional Comments:
Good use of AI-generated tests; monitor closely.

Copy link
Contributor

@barkangel barkangel 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 easily readable and easy to understand especially in app/src/tests/FontSizeContext.test.js

  • Code Quality:
    Code is neat and the testing descriptions are applicable to what is being tested.

  • Testing:
    Very nice job with the tests boosting the overall test percentage!

  • Functionality:
    Tests are functional and well made.

  • Housekeeping:
    N/A

Additional Comments:

ChatGPT usage is noted and documented in-file in the top 2 comments of said page, and also clarified in the pull request. Nice work!

@thegreatzoidberg
Copy link
Contributor

Pull Request Review:

  • Readability:

    • Code is readable, comments made to help reader.
    • Comment link to ChatGPT conversation on line 2 of SettingsPage.test.js.
  • Code Quality:

    • Code looks good to me and passes all checks.
  • Testing:

    • One more test file, one test file modified.
    • All checks passed.
  • Functionality:

    • Both FontSizeContext.js and SettingsPage.js fully covered.
  • Housekeeping:

Additional Comments:

  • Let's double check how the professor wants us to describe AI usage. It was just for the Design Portfolio right?

@DillonDavidson DillonDavidson merged commit e7163aa 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.

4 participants