webcompat.com icon indicating copy to clipboard operation
webcompat.com copied to clipboard

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

Open karlcow opened this issue 5 years ago • 9 comments

https://github.com/webcompat/webcompat.com/blob/136607eeb2ff0c65ee681ad942875937a920cb9b/webcompat/api/endpoints.py#L30

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

karlcow avatar Feb 12 '20 07:02 karlcow

since 2016, the code has a blueprint for errors which is… not used. https://github.com/webcompat/webcompat.com/commit/44e804ff3d99ed389c996c4f745049717dbbff3c

karlcow avatar May 14 '20 08:05 karlcow

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

miketaylr avatar May 14 '20 15:05 miketaylr

@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.

karlcow avatar May 14 '20 20:05 karlcow

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 avatar May 14 '20 20:05 miketaylr

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 avatar May 14 '20 21:05 karlcow

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 avatar May 14 '20 23:05 karlcow

@miketaylr ah just realized this

https://github.com/webcompat/webcompat.com/blob/ad693004dcc017fc56b7a2e85f287948db0075a2/tests/unit/test_api_urls.py#L54

And now trying to understand we decided to do that.

karlcow avatar May 15 '20 00:05 karlcow

Maybe this needs to be revisited. https://github.com/webcompat/webcompat.com/issues/712#issuecomment-147748826

karlcow avatar May 15 '20 00:05 karlcow

That was previous to the work done by bea in https://github.com/webcompat/webcompat.com/pull/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.

karlcow avatar May 15 '20 00:05 karlcow