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

Jump to bottom

Get endpoint uri test #42

Merged
merged 11 commits into from
Apr 24, 2024
Merged

Get endpoint uri test #42

Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
63ea2e5
Added endpoint uri testing
adrianvrj Apr 22, 2024
8f9a3eb
Added schema and correct patch method
adrianvrj Apr 23, 2024
ccd9fd1
Update test_utils.py
adrianvrj Apr 23, 2024
03ad784
Merge branch 'gizatechxyz:main' into test-utils
adrianvrj Apr 23, 2024
df54f72
added return value inside functions for readability
adrianvrj Apr 23, 2024
9e4443e
Merge branch 'test-utils' of https://github.com/adrianvrj/actions-sdk…
adrianvrj Apr 23, 2024
08cf401
pre-commit changes in test_utils
adrianvrj Apr 23, 2024
6f6974c
run pre-commit correctly
adrianvrj Apr 23, 2024
d3bd146
Correctly import patch
adrianvrj Apr 23, 2024
8d3970f
Merge branch 'main' into test-utils
adrianvrj Apr 24, 2024
b9fdcf9
pre-commit run after solving conflicts
adrianvrj Apr 24, 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
25 changes: 25 additions & 0 deletions tests/test_utils.py
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

This file is missing formatting, this should have been handled by pre-commit, make sure it is installed and run pre-commit run --files tests/test_utils.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already done this, you can see it at 08cf401 commit

Copy link
Contributor

Choose a reason for hiding this comment

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

Running pre-commit locally yields changes:

Check Yaml...........................................(no files to check)Skipped
Fix End of Files.........................................................Passed
Trim Trailing Whitespace.................................................Passed
Check for added large files..............................................Passed
Check for merge conflicts................................................Passed
Don't commit to branch...................................................Passed
pyupgrade................................................................Passed
isort....................................................................Failed
- hook id: isort
- files were modified by this hook

Fixing /Users/gonmeso/src/external/actions-sdk/tests/test_utils.py

black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted tests/test_utils.py

All done! ✨ 🍰 ✨
1 file reformatted.

ruff.....................................................................Failed
- hook id: ruff
- exit code: 1

tests/test_utils.py:1:1: F403 `from unittest.mock import *` used; unable to detect undefined names
tests/test_utils.py:3:8: F401 [*] `pytest` imported but unused
tests/test_utils.py:9:2: F405 `patch` may be undefined, or defined from star imports
tests/test_utils.py:29:2: F405 `patch` may be undefined, or defined from star imports
Found 4 errors.
[*] 1 fixable with the `--fix` option.

Run pre-commit run --files tests/test_utils.py and fix the formating and linting issues

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think now I get it right

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
from unittest.mock import *
Copy link
Contributor

Choose a reason for hiding this comment

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

Importing with * is considered a bad practice and usually discouraged in lintiing tools, only import what is needed, which is patch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed here d3bd146

import pytest
from giza.schemas.endpoints import Endpoint, EndpointsList
from giza_actions.utils import get_endpoint_uri

endpoint_data = Endpoint(id=999, size="S", is_active=True, model_id=999, version_id=999, uri="testing.uri")
endpoint_list = EndpointsList(root=[endpoint_data])
Copy link
Contributor

Choose a reason for hiding this comment

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

To improve readability a bit we could add this inside the test and add it to the mock:

@patch("giza.client.EndpointsClient.list")
def test_get_endpoint_uri_successful(mock_get):
    """
    Tests successful retrieval of the deployment URI for a model and version.
    """
    endpoint_data = Endpoint(id=999, size="S", is_active=True, model_id=999, version_id=999, uri="testing.uri")
    endpoint_list = EndpointsList(root=[endpoint_data])
    mock_get.return_value = endpoint_list
    uri = get_endpoint_uri(model_id=788, version_id=23)
    assert uri is "testing.uri"
    mock_get.assert_called_once()

This way data that its only for a test its contained inside and not for the whole script. Make sure to change the test_get_endpoint_uri_not_found test as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already done this too, it is the df54f72 commit

@patch("giza.client.EndpointsClient.list", return_value=endpoint_list)
def test_get_endpoint_uri_successful(mock_get):
"""
Tests successful retrieval of the deployment URI for a model and version.
"""
uri = get_endpoint_uri(model_id=788, version_id=23)
assert uri is "testing.uri"
mock_get.assert_called_once()

endpoint_list = EndpointsList(root=[])
@patch("giza.client.EndpointsClient.list", return_value=endpoint_list)
def test_get_endpoint_uri_not_found(mock_get):
"""
Tests the case where no active deployment is found for the model and version.
"""
uri = get_endpoint_uri(model_id=516, version_id=19)
assert uri is None
mock_get.assert_called_once()
Loading