Skip to content

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

Merged
karlcow merged 4 commits intowebcompat:masterfrom
karlcow:3321/1
Jun 1, 2020
Merged

Fixes #3321 - Converts to pytest and improves coverage#3322
karlcow merged 4 commits intowebcompat:masterfrom
karlcow:3321/1

Conversation

@karlcow
Copy link
Copy Markdown
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 ksy36 and miketaylr May 29, 2020 08:20
@karlcow karlcow mentioned this pull request May 29, 2020
6 tasks
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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