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

Fixes #3321 - Converts to pytest and improves coverage #3322

Merged
merged 4 commits into from
Jun 1, 2020

Conversation

karlcow
Copy link
Member

@karlcow karlcow commented May 29, 2020

This PR fixes issue #3321

Proposed PR background

Please provide enough information so that others can review your pull request:

This took a bit more time than I was expecting but I have figured out a couple of things.
This also should lead to more test organization. I need to open an issue so that we put the mock_api_response outside of test_api_urls. (probably in conftest.py if I understood well pytest)

Anyway.

It basically

  1. Converts the test for test_issues.py to pytest
  2. Adds missing tests for cases which were not covered in the tests (thanks to coverage for finding them.)
  3. Modifies mock_api_response to add .json() method for the mocks.

Now we have 100% coverage for the issues.py module. See #3205 for the work to do still.

(env) ~/code/webcompat.com % COLUMNS=50 coverage report -m                                            
Name                                   Stmts   Miss  Cover   Missing
--------------------------------------------------------------------
…
webcompat/issues.py                       48      0   100%
…
--------------------------------------------------------------------
TOTAL                                   1897    343    82%

ok let's see if we stay green or if we broke something.

karlcow added 2 commits May 29, 2020 17:03
That will make testing easier in certain casesby giving a github_data mock which has a meaning github_data.json() function.
This simplifies a bit the tests and it exposes only what we need in this context.
@karlcow karlcow requested review from miketaylr and ksy36 May 29, 2020 08:20
@karlcow karlcow mentioned this pull request May 29, 2020
6 tasks
Copy link
Member

@miketaylr miketaylr left a comment

Choose a reason for hiding this comment

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

Looks good! Just a few minor changes.

1. It converts all the tests to pytest format
2. It adds new tests for handling the scenario which were not previously tested (as revealed by coverage).

issues.py is now 100% covered.
@karlcow
Copy link
Member Author

karlcow commented Jun 1, 2020

ok rebased. and pushed.

@karlcow karlcow requested a review from miketaylr June 1, 2020 01:01
Copy link
Member

@miketaylr miketaylr left a comment

Choose a reason for hiding this comment

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

LGTM. Feel free to rename the client method or not, up to you.

@karlcow karlcow merged commit f88375b into webcompat:master Jun 1, 2020
@miketaylr
Copy link
Member

Thanks Karl, very nice improvements.

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.

3 participants