test: Add tests for Users with mock responses #46
Conversation
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
| @responses.activate | ||
| def test_get_users(self): | ||
| responses.get( | ||
| "https://connect.example/__api__/v1/users", |
There was a problem hiding this comment.
I made these fixtures by querying a real Connect and then sanitizing the responses.
src/posit/connect/users_test.py
Outdated
| # TODO: page_size should go with find(), can't pass it to client.users | ||
| u = Users(con.config, con.session, page_size=2) | ||
| # TODO: Add __len__ method to Users | ||
| assert len(u.find().data) == 3 |
There was a problem hiding this comment.
What I want this to be is:
u = con.users.find(page_size=2)
assert len(u) == 3
There was a problem hiding this comment.
I was thinking about making page_size a Config property. There are a handful of endpoints that support paging. Placing the value in Config will make it accessible to all future implementations of Resources.
con = Client(api_key="12345", url="https://connect.example/", page_size=2)
u = con.users.find()
assert len(u) == 3
Adding page_size to find will require more refactoring.
There was a problem hiding this comment.
Adding
page_sizetofindwill require more refactoring.
Yeah that's why I didn't just do it yesterday 😂 I had an idea while walking the dog though, may give it a shot later.
There was a problem hiding this comment.
😄 Sounds good!
For what it's worth, I'm thinking through another version of the Resources base class that will do a better job of handling these situations.
There was a problem hiding this comment.
Yeah, my other thought was to ship this, build (or more likely let you build) the Content classes, and see how some of this shakes out when we have two kinds of Resources to work with.
There was a problem hiding this comment.
I made some issues for these and linked the issue numbers in the comment (i.e. # TODO(#47)) so that we can find them later.
| @@ -6,3 +6,4 @@ responses>=0.25 | |||
| ruff==0.1.14 | |||
| setuptools==69.0.3 | |||
| setuptools-scm==8.0.4 | |||
| pandas | |||
There was a problem hiding this comment.
No version pin here bc for our purposes, it doesn't matter, and because the current release of pandas isn't available for Python 3.8.
|
I'm going to merge this: it pushes test coverage to 96% of lines, and it covers the key functionality of the Users resources. I want to try a refactor and need this coverage in place first. |
Plus some TODOs that come out of this (I'll come back to them tomorrow, or you can feel free to push to this branch). This should exercise some of the previously untested code, and should open up a path for adding some enhancements on top.