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.

Sign up for GitHub

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

test: Add tests for Users with mock responses #46

Merged
merged 6 commits into from
Feb 23, 2024
Merged

test: Add tests for Users with mock responses #46

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6f6366b
Add a test with responses for users.find
nealrichardson Feb 21, 2024
b3db2c4
Add TODOs
nealrichardson Feb 21, 2024
a3d63df
Add tests for to_pandas and find_one
nealrichardson Feb 22, 2024
ac5d027
Fix CI
nealrichardson Feb 22, 2024
804ac67
test get user by id
nealrichardson Feb 23, 2024
40e03d7
Remove unused import
nealrichardson Feb 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions src/posit/connect/users_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import pytest
import responses

from unittest.mock import patch

from .client import Client
from .users import Users


Expand All @@ -21,3 +23,79 @@ class TestUsers:
def test_init(self, mock_config, mock_session):
with pytest.raises(ValueError):
Users(mock_config, mock_session, page_size=9999)

@responses.activate
def test_get_users(self):
responses.get(
"https://connect.example/__api__/v1/users",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I made these fixtures by querying a real Connect and then sanitizing the responses.

match=[
responses.matchers.query_param_matcher(
{"page_size": 2, "page_number": 1}
)
],
json={
"results": [
{
"email": "[email protected]",
"username": "al",
"first_name": "Alice",
"last_name": "User",
"user_role": "publisher",
"created_time": "2017-08-08T15:24:32Z",
"updated_time": "2023-03-02T20:25:06Z",
"active_time": "2018-05-09T16:58:45Z",
"confirmed": True,
"locked": False,
"guid": "a01792e3-2e67-402e-99af-be04a48da074",
},
{
"email": "[email protected]",
"username": "robert",
"first_name": "Bob",
"last_name": "Loblaw",
"user_role": "publisher",
"created_time": "2023-01-06T19:47:29Z",
"updated_time": "2023-05-05T19:08:45Z",
"active_time": "2023-05-05T20:29:11Z",
"confirmed": True,
"locked": False,
"guid": "87c12c08-11cd-4de1-8da3-12a7579c4998",
},
],
"current_page": 1,
"total": 3,
},
)
responses.get(
"https://connect.example/__api__/v1/users",
match=[
responses.matchers.query_param_matcher(
{"page_size": 2, "page_number": 2}
)
],
json={
"results": [
{
"email": "[email protected]",
"username": "carlos12",
"first_name": "Carlos",
"last_name": "User",
"user_role": "publisher",
"created_time": "2019-09-09T15:24:32Z",
"updated_time": "2022-03-02T20:25:06Z",
"active_time": "2020-05-11T16:58:45Z",
"confirmed": True,
"locked": False,
"guid": "20a79ce3-6e87-4522-9faf-be24228800a4",
},
],
"current_page": 2,
"total": 3,
},
)

con = Client(api_key="12345", url="https://connect.example/")
# 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
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What I want this to be is:

u = con.users.find(page_size=2)
assert len(u) == 3

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding page_size to find will 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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

😄 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

Loading