Page MenuHomeSoftware Heritage

add tests for parameter validation
ClosedPublic

Authored by zack on Nov 13 2019, 9:35 AM.

Diff Detail

Repository
rDGRPH Compressed graph representation
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz added a subscriber: vlorentz.

Could you make the tests check for the error message? (with raises(RemoteException, match='regexp'):)

This revision now requires changes to proceed.Nov 13 2019, 12:25 PM

Could you make the tests check for the error message? (with raises(RemoteException, match='regexp'):)

I really think that's an antipattern. If anything, we should stop doing it elsewhere rather than adding it in more places.
If the type of error matters (which is arguably the case here), it should be captured by exception attributes other than a human readable message, e.g., exception type, exception properties, etc.

  • client.py: renamse check_status -> raise_for_status
  • test_api_client: check HTTP status codes for bad requests

That would be ok with me.

Now done. With the last changes in D2266 we can now check that the returned HTTP status codes here are correct.

This revision is now accepted and ready to land.Nov 18 2019, 9:02 AM
  • add tests for parameter validation
  • client.py: renamse check_status -> raise_for_status
  • test_api_client: check HTTP status codes for bad requests
  • add tests for parameter validation
  • client.py: renamse check_status -> raise_for_status
  • test_api_client: check HTTP status codes for bad requests
  • test_api_client.py: do not test /walk parameter validation