Skip to content

Conversation

@irisbian504
Copy link
Collaborator

@irisbian504 irisbian504 commented Apr 10, 2025

Fixes #2393

@codecov
Copy link

codecov bot commented Apr 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.40%. Comparing base (58241ba) to head (040e4c2).
Report is 114 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2363      +/-   ##
==========================================
- Coverage   77.16%   76.40%   -0.77%     
==========================================
  Files          88       92       +4     
  Lines        3377     3607     +230     
  Branches      366      404      +38     
==========================================
+ Hits         2606     2756     +150     
- Misses        703      783      +80     
  Partials       68       68              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@anth-volk anth-volk left a comment

Choose a reason for hiding this comment

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

@irisbian504 This looks great! I have a couple nits highlighted. Otherwise, could you create an issue to add tests for the update_profile method, then add "Fixes #XXXX" in the PR description?


# WHEN we call update_profile with new data
updated_username = "updated_username"
updated_country = "UK"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Technically, this should be lowercase

# WHEN we call update_profile for this nonexistent record
result = service.update_profile(
user_id=NONEXISTENT_ID,
primary_country="UK",
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Here, too, this would be lowercase in production

Copy link
Collaborator

@anth-volk anth-volk left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @irisbian504

@anth-volk anth-volk merged commit bfc2131 into master May 17, 2025
6 of 7 checks passed
@anth-volk anth-volk deleted the Iris_update_profile_test branch May 17, 2025 21:06
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.

Added new test for update_profile method for user service

3 participants