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.

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

adding tests to utils.py #45

Merged
merged 4 commits into from
Apr 26, 2024

Conversation

stevencartavia
Copy link

Added tests for the read_json function from the utils.py file.



@mock.patch("builtins.open")
@mock.patch("json.load", mock.MagicMock(side_effect=[{"test": "test"}]))
Copy link
Contributor

Choose a reason for hiding this comment

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

MagicMock is automatically used by patch, just add directly the side effect:

@mock.patch("json.load", side_effect=[{"test": "test"}])

Copy link
Contributor

@Gonmeso Gonmeso left a comment

Choose a reason for hiding this comment

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

Thank you for testing read_json just a slight change and it will be good to goo!

"""
Tests when the JSON file is not found.
"""
with TestCase.assertRaises(TestCase, FileNotFoundError):
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use pytest.raises (docs):

with pytest.raises(FileNotFoundError):
...

If you could also make this change for line 67 it will be greatly appreciated

@stevencartavia stevencartavia changed the title stevencartavia:test-utils test-utils Apr 26, 2024
@stevencartavia stevencartavia changed the title test-utils adding tests to utils.py Apr 26, 2024
@Gonmeso
Copy link
Contributor

Gonmeso commented Apr 26, 2024

@stevencartavia CI is failing, please check the errors and fix them

Copy link
Contributor

@Gonmeso Gonmeso left a comment

Choose a reason for hiding this comment

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

LGTM!

@Gonmeso Gonmeso merged commit f05823c into gizatechxyz:main Apr 26, 2024
1 check passed
@Gonmeso Gonmeso mentioned this pull request Apr 26, 2024
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.

2 participants