tophat / codewatch

[deprecated] Monitor and manage deeply customizable metrics about your python code using ASTs
https://codewatch.io
Apache License 2.0
38 stars 3 forks source link

Proposal: Use AssertionError in @assertion functions #61

Closed francoiscampbell closed 5 years ago

francoiscampbell commented 5 years ago

Proposal implementation for #9. This changes the tuple return of @assertion functions to raise AssertionError in the case of a failed assertion. If the function does not raise anything, the assertion passes. If it raises any other error, the error is recorded as being distinct from a failure.

The benefits are:

  1. Raising exceptions in error situations is more pythonic than returning a result, err tuple
  2. But the syntax is almost the same; in most cases, changing return to assert is sufficient
  3. It is more consistent with Python unit testing frameworks
  4. It enables the use of the assert keyword in @assertion functions
  5. To make the assertion checker resilient against programming error, we have to make it catch Exceptions anyways (otherwise an exception like a KeyError would stop all the remaining @assertion functions from running)

The disadvantage is that only a single failure per @assetion can be reported, as the @assertion will terminate upon raising its first AssertionError. This is not a major problem, as it is consistent with other testing frameworks, it is a limitation of the current return result, err pattern, and failing early encourages the writing of short and simple @assertion functions.

codecov[bot] commented 5 years ago

Codecov Report

Merging #61 into master will increase coverage by 0.06%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
+ Coverage   96.33%   96.39%   +0.06%     
==========================================
  Files           8        8              
  Lines         327      333       +6     
  Branches       46       46              
==========================================
+ Hits          315      321       +6     
  Misses          6        6              
  Partials        6        6
Flag Coverage Δ
#py27 96.39% <100%> (+0.06%) :arrow_up:
#py36 96.39% <100%> (+0.06%) :arrow_up:
#py37 96.39% <100%> (+0.06%) :arrow_up:
Impacted Files Coverage Δ
codewatch/assertion.py 100% <100%> (ø) :arrow_up:
codewatch/stats.py 100% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c3af370...0d5ae00. Read the comment docs.

francoiscampbell commented 5 years ago

Looks great, version bump would be good for this

Yup, I agree. Do I need to do it in the PR or is it done after?

lime-green commented 5 years ago

Either works. Although the github release has to be done manually (just through the UI)

So the flow would be

francoiscampbell commented 5 years ago

Since the release is manual, i'll leave the version bump out of the PR.