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

Raise an exception instead of swallowing an error #126

Merged
merged 3 commits into from Jun 28, 2021

Conversation

luandy64
Copy link
Contributor

Description of change

This PR highlights and fixes an edge case in raise_for_error()

Manual QA steps

Risks

Rollback steps

  • revert this branch

@luandy64
Copy link
Contributor Author

$ pytest -s tests/unittests/test_exception_handling.py::TestBug
===================================== test session starts ======================================
platform linux -- Python 3.8.10, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /home/vagrant/git/tap-github
collected 1 item

tests/unittests/test_exception_handling.py INFO METRIC: {"type": "timer", "metric": "http_request_duration", "value": 3.24249267578125e-05, "tags": {"endpoint": "teams", "status": "succeeded"}}
INFO METRIC: {"type": "counter", "metric": "record_count", "value": 0, "tags": {"endpoint": "teams"}}
F

=========================================== FAILURES ===========================================
___________________________ TestBug.test_for_bug_in_raise_for_error ____________________________

self = <test_exception_handling.TestBug testMethod=test_for_bug_in_raise_for_error>
mocked_request = <MagicMock name='request' id='140288628972368'>

    def test_for_bug_in_raise_for_error(self, mocked_request):
        mocked_request.return_value = get_response(400, raise_error = True, content='')

        with self.assertRaises(tap_github.GithubException):
>           [x for x in tap_github.get_all_teams(None, "", None, None, None)]

tests/unittests/test_exception_handling.py:33:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

    def get_all_teams(schemas, repo_path, state, mdata, _start_date):
        org = repo_path.split('/')[0]
        with metrics.record_counter('teams') as counter:
            for response in authed_get_all_pages(
                    'teams',
                    'https://api.github.com/orgs/{}/teams?sort=created_at&direction=desc'.format(org)
            ):
>               teams = response.json()
E               AttributeError: 'NoneType' object has no attribute 'json'

tap_github/__init__.py:358: AttributeError
=================================== short test summary info ====================================
FAILED tests/unittests/test_exception_handling.py::TestBug::test_for_bug_in_raise_for_error

@hpatel41 hpatel41 marked this pull request as ready for review June 28, 2021 07:41
@KAllan357 KAllan357 merged commit 778682b into master Jun 28, 2021
@KAllan357 KAllan357 deleted the raise_for_error-edge-case branch June 28, 2021 19:00
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.

None yet

4 participants