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

[test] api/endpoints.py needs more tests coverage #3204

Open
karlcow opened this issue Feb 12, 2020 · 9 comments
Open

[test] api/endpoints.py needs more tests coverage #3204

karlcow opened this issue Feb 12, 2020 · 9 comments

Comments

@karlcow
Copy link
Member

karlcow commented Feb 12, 2020

api = Blueprint('api', __name__, url_prefix='/api')

The API is only partially tested currently. (69% as of today) and would benefit of a better coverage.

karlcow added a commit to karlcow/webcompat.com that referenced this issue May 14, 2020
The fact we need to mock that much is probably a sign
that the application is not simple enough
and would deserve some refactoring.

Probably the return should be at the end and handle all the cases,
once all the parameters have been defined.
karlcow added a commit to karlcow/webcompat.com that referenced this issue May 14, 2020
Our function needs probably to be refactored so that
it becomes easier to test. See previous commit.
@karlcow
Copy link
Member Author

karlcow commented May 14, 2020

since 2016, the code has a blueprint for errors which is… not used.
44e804f

@miketaylr
Copy link
Member

Unused means no errors caused by this code, right???

@karlcow
Copy link
Member Author

karlcow commented May 14, 2020

@miketaylr I don't think any current views are returning something because of this blueprint, or said in anothe way, it seems we could delete it without any impact, I haven't tried it yet.

@miketaylr
Copy link
Member

Are we sure this isn't actually used? AFAICT, this is the only thing controlling the error messages that we do see (like Not Found. Lost in Punk Cat Space). => https://webcompat.com/tacos

@miketaylr miketaylr reopened this May 14, 2020
@karlcow
Copy link
Member Author

karlcow commented May 14, 2020

ah right then it's used! And it's magical and I need to double check something. Thanks for digging this.

This is working indeed! Cool.

http GET https://webcompat.com/api/tacos 'Accept:application/json'
HTTP/1.1 404 NOT FOUND
Connection: keep-alive
Content-Length: 61
Content-Security-Policy: default-src 'self'; object-src 'none'; connect-src 'self' https://api.github.com; font-src 'self' https://fonts.gstatic.com; img-src 'self' https://www.google-analytics.com https://*.githubusercontent.com data:; manifest-src 'self'; script-src 'self' https://www.google-analytics.com https://api.github.com 'nonce-d054ca11323e0f9e2725f2b8121d14960c98e8af'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; base-uri 'self'; frame-ancestors 'self'; report-uri /csp-report
Content-Type: application/json
Date: Thu, 14 May 2020 21:19:39 GMT
Server: nginx/1.14.0 (Ubuntu)
Set-Cookie: exp=v2; Expires=Thu, 21-May-2020 21:19:39 GMT; Max-Age=604800; Path=/
Strict-Transport-Security: max-age=31536000; includeSubDomains;
X-Content-Type-Options: nosniff
X-Frame-Options: DENY
X-XSS-Protection: 1; mode=block

{
    "message": "Not Found. Lost in Punk Cat Space",
    "status": 404
}

That will simplify my tests reasoning and removes one modification I did yesterday. My test was bad.

karlcow added a commit to karlcow/webcompat.com that referenced this issue May 14, 2020
@karlcow
Copy link
Member Author

karlcow commented May 14, 2020

one thing i like about the combination of pytest and coverage is that it really shows what is not tested.

webcompat/api/endpoints.py               100     22    78%   110-117, 128-130, 132-133, 163-169, 182-183, 201-203

@karlcow
Copy link
Member Author

karlcow commented May 15, 2020

@miketaylr ah just realized this

webcompat.app.config['TESTING'] = False

And now trying to understand we decided to do that.

@karlcow
Copy link
Member Author

karlcow commented May 15, 2020

Maybe this needs to be revisited.
#712 (comment)

@karlcow
Copy link
Member Author

karlcow commented May 15, 2020

That was previous to the work done by bea in #1588

And we could revert the decision of webcompat.app.config['TESTING'] = False and adjust the tests. That seems not good to make a special case of api.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants