yarikoptic commented 4 years ago

RuntimeError is too generic and could be raise for too many other reasons.

E.g. (not sure if could be raised here) tests already test for some other RuntimeErrors rising:

def test_etrequest():
    endpoint = "http://fakeendpoint/"
    with pytest.raises(RuntimeError):
        _etrequest(endpoint, method="get")
    assert _etrequest(ET_ROOT)
    # ensure timeout is working properly
    endpoint = "https://google.com"
    with pytest.raises(RuntimeError):
        _etrequest(endpoint, timeout=0.01)
    assert _etrequest(endpoint)

satra commented 4 years ago

@yarikoptic - could you please merge/rebase on master

yarikoptic commented 4 years ago

done! although I think I was first (before all the master changes of today :-P)

codecov[bot] commented 4 years ago

Codecov Report

Merging #23 into master will increase coverage by 0.53%. The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master      #23      +/-   ##
==========================================
+ Coverage   81.15%   81.69%   +0.53%     
==========================================
  Files           3        3              
  Lines          69       71       +2     
==========================================
+ Hits           56       58       +2     
  Misses         13       13

Flag	Coverage Δ
#unittests	`81.69% <66.66%> (+0.53%)`	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
etelemetry/client.py	`80.95% <66.66%> (+0.62%)`	: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 5115137...fa9a06c. Read the comment docs.

sensein / etelemetry-client

ENH: dedicated BadVersionError to raise if bad version detected #23

Codecov Report