Skip to content

Comments

IMPROVE - Add test coverage for OdometerViewModel#380

Open
lauriecai wants to merge 20 commits intomikaelacaron:devfrom
lauriecai:379-odometer-unit-tests
Open

IMPROVE - Add test coverage for OdometerViewModel#380
lauriecai wants to merge 20 commits intomikaelacaron:devfrom
lauriecai:379-odometer-unit-tests

Conversation

@lauriecai
Copy link

@lauriecai lauriecai commented Feb 26, 2025

What it Does

Resolves #379 by validating CRUD operations for OdometerViewModel

  • Tests initial state, add/delete/get/update odometer readings, and get vehicles

How I Tested

  • Set useEmulator to true within OdometerViewModelTests init function
  • Initialize each test with a userUID, viewModel, and newReading
  • Test CRUD operations by adding, deleting, updating, and fetching newReading, then checking database for newReading

Notes

  • This is my first contribution in a codebase that isn't my own and is this large - would appreciate any feedback with regard to approach, things to consider, alternatives, folder structure, etc.
  • Is issue the same as a PR? I created a new issue IMPROVE - Add test coverage for OdometerViewModel #379, but this ticket contains more information about what was done. Is this redundant or meant to be separate? Is there a better way to link the two?

@lauriecai lauriecai changed the title IMPROVE - Add test coverage for OdometerViewModel #379 IMPROVE - Add test coverage for OdometerViewModel Feb 26, 2025
@mikaelacaron
Copy link
Owner

mikaelacaron commented Mar 2, 2025

Thank you for contributing!

The point of the issue is because it's more of the idea of "what are you planning to do" it generally doesn't have the exact specifics of how to complete that issue

The PR is more about describing what exactly you did and why. (Because it's closer related to the git history and will have details that may be helpful if there's a bug found in the future related to this PR)

So how you wrote them is fine! I'm about to review the PR itself, so I may make more notes, but generally for this project you should be using the Firebase emulator. Is there anything in the CONTRIBUTING instructions that I can add that makes it easier to understand what it's for and why you should be using it when contributing to this project?

@lauriecai
Copy link
Author

lauriecai commented Mar 2, 2025

Thanks for the explanation - I'll do some more investigation on Firebase emulator and how to write better tests around that (instructions were clear, I was just worried about biting off more than I could chew). Will come back with a proposed plan. If you do end up with some feedback before that, please share!

Copy link
Owner

@mikaelacaron mikaelacaron left a comment

Choose a reason for hiding this comment

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

Great work to start this! I've left several comments some about style, but some about how you implemented the unit tests.

If you have any questions, you can comment directly on that thread, otherwise click the resolve button once you've addressed my comment. Once you fix everything, please click the re-review button

Re-review button

@lauriecai lauriecai requested a review from mikaelacaron March 18, 2025 22:36
@mikaelacaron
Copy link
Owner

Hey @lauriecai ! I appreciate all the work you did on this PR forever ago! Hope you're doing well. I'm updating it to get it merged (just so you know, if you see any notifications about it)

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.

IMPROVE - Add test coverage for OdometerViewModel

2 participants