-
Notifications
You must be signed in to change notification settings - Fork 4
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
chore: Move tests to tests/ dir #60
Changes from all commits
17cffa7
69906b5
9821de9
df1e455
1a2e5f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,25 +22,21 @@ jobs: | |
with: | ||
python-version: ${{ matrix.python-version }} | ||
- run: make deps | ||
- run: make install | ||
- run: make test | ||
# Run coverage on one of the builds, doesn't matter which | ||
- if: ${{ matrix.python-version == '3.12' }} | ||
run: make cov-xml | ||
- if: ${{ matrix.python-version == '3.12' }} | ||
uses: orgoro/[email protected] | ||
with: | ||
coverageFile: coverage.xml | ||
thresholdAll: 0.8 | ||
token: ${{ secrets.GITHUB_TOKEN }} | ||
lint: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: actions/setup-python@v5 | ||
- run: make deps | ||
- run: make lint | ||
cov: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: actions/setup-python@v5 | ||
- run: make deps | ||
- run: make test | ||
- run: make cov-xml | ||
- if: always() | ||
uses: orgoro/[email protected] | ||
with: | ||
coverageFile: coverage.xml | ||
thresholdAll: 0.8 | ||
token: ${{ secrets.GITHUB_TOKEN }} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,9 +27,8 @@ Source = "https://github.com/posit-dev/posit-sdk-py" | |
Issues = "https://github.com/posit-dev/posit-sdk-py/issues" | ||
|
||
[tool.pytest.ini_options] | ||
python_files = [ | ||
"*_test.py", | ||
"*_test_*.py", | ||
addopts = [ | ||
"--import-mode=importlib", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you verified that this gives you the functionality you want? https://docs.pytest.org/en/7.1.x/explanation/pythonpath.html?highlight=import%20mode#import-modes
If I understand this correctly, this means that abstracting mocked classes into their own module won't work. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have not verified. I was just blindly following the pytest docs recommendation: "For new projects, we recommend to use importlib import mode." Happy to remove if/when it's a problem. |
||
] | ||
|
||
[tool.setuptools_scm] | ||
|
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import responses | ||
|
||
from .client import Client | ||
from posit.connect.client import Client | ||
|
||
|
||
class TestContents: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
import io | ||
|
||
import pytest | ||
|
||
from requests import HTTPError, Response | ||
from unittest.mock import Mock, patch | ||
|
||
from posit.connect.errors import ClientError | ||
from posit.connect.hooks import handle_errors | ||
|
||
|
||
def test_success(): | ||
response = Mock() | ||
response.status_code = 200 | ||
assert handle_errors(response) == response | ||
|
||
|
||
def test_client_error(): | ||
response = Mock() | ||
response.status_code = 400 | ||
response.json = Mock(return_value={"code": 0, "error": "foobar"}) | ||
with pytest.raises(ClientError): | ||
handle_errors(response) | ||
|
||
|
||
@patch("posit.connect.hooks.JSONDecodeError") | ||
def test_client_error_without_payload(JSONDecodeError): | ||
response = Mock() | ||
response.status_code = 404 | ||
response.json = Mock(side_effect=JSONDecodeError()) | ||
response.raise_for_status = Mock(side_effect=Exception()) | ||
with pytest.raises(Exception): | ||
handle_errors(response) | ||
|
||
|
||
def test_200(): | ||
response = Response() | ||
response.status_code = 200 | ||
assert handle_errors(response) == response | ||
|
||
|
||
def test_response_client_error_with_plaintext_payload(): | ||
response = Response() | ||
response.status_code = 404 | ||
response.raw = io.BytesIO(b"Plain text 404 Not Found") | ||
with pytest.raises(HTTPError): | ||
handle_errors(response) | ||
|
||
|
||
def test_response_client_error_with_json_payload(): | ||
response = Response() | ||
response.status_code = 400 | ||
response.raw = io.BytesIO(b'{"code":0,"error":"foobar"}') | ||
with pytest.raises( | ||
ClientError, match=r"foobar \(Error Code: 0, HTTP Status: 400 Bad Request\)" | ||
): | ||
handle_errors(response) | ||
|
||
|
||
def test_response_client_error_without_payload(): | ||
response = Response() | ||
response.status_code = 404 | ||
response.raw = io.BytesIO(b"Plain text 404 Not Found") | ||
with pytest.raises(HTTPError): | ||
handle_errors(response) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import responses | ||
|
||
from .client import Client | ||
from posit.connect import Client | ||
|
||
|
||
class TestOAuthIntegrations: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to keep
- if: always()
here. Otherwise, coverage won't report ifmake cov-xml
fails.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you want
if: always
because it's only running on one branch of the matrix. And anyway, ifmake cov-xml
fails, how can coverage report?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, should it be...?
- if: ${{ !cancelled() && matrix.python-version == '3.12' }}
https://docs.github.com/en/actions/learn-github-actions/expressions#status-check-functions
make cov-xml
will fail if the coverage is below report requirements but will still report the coverage. We want this step to run regardless so that the coverage report is added to the Pull Request.Here is an example of the behavior: #67
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, got it. Thanks for the demonstration.
Would it be better to just remove
fail_under = 80
from .coveragerc? I'm not sure it's helping us more than having the coverage report on the PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either works for me.