vheon / JediHTTP

Simple http wrapper around jedi
Apache License 2.0
40 stars 9 forks source link

[READY] Enable coverage #41

Closed micbou closed 7 years ago

micbou commented 7 years ago

Use codecov for coverage reports. Fix thread coverage by updating Waitress dependency to 0.8.10 (see this commit message).


This change is Reviewable

codecov-io commented 7 years ago

Codecov Report

Merging #41 into master will not change coverage. The diff coverage is 100%.

@@           Coverage Diff           @@
##           master      #41   +/-   ##
=======================================
  Coverage   94.48%   94.48%           
=======================================
  Files           8        8           
  Lines         308      308           
=======================================
  Hits          291      291           
  Misses         17       17
micbou commented 7 years ago

Took me some time but the OS X builds are now successfully uploading the coverage reports. The issue was that the install.sh script was not sourced and so it wasn't using the Python where coverage.py was installed when trying to upload the report.

Also, I removed the virtualenv part (and the run.sh script) since tox is already creating a virtual environment.


Reviewed 14 of 14 files at r1. Review status: all files reviewed at latest revision, all discussions resolved, some commit checks broke.


Comments from Reviewable

vheon commented 7 years ago

Reviewed 14 of 14 files at r1. Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks broke.


jedihttp/main.py, line 11 at r1 (raw file):

# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and

Isn't this renamed file a breaking change? What benefit it will bring, other than it looks more Pythonic? I'm not saying that I'm against it, just checking :confused:


Comments from Reviewable

micbou commented 7 years ago

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks broke.


jedihttp/main.py, line 11 at r1 (raw file):

Previously, vheon (Andrea Cedraro) wrote…
Isn't this renamed file a breaking change? What benefit it will bring, other than it looks more Pythonic? I'm not saying that I'm against it, just checking :confused:

I couldn't find a way to configure nosetests coverage without renaming this file. The issue is that nosetests considers jedihttp/__init__.py and jedihttp.py to be the same file and thus it sees jedihttp.py as an empty file. With all files put in the jedihttp directory, it's just a matter of adding the --cover-module=jedihttp option to the nosetests call. Also, as you said, it's a more standard way of structuring a Python package.


Comments from Reviewable

vheon commented 7 years ago

:lgtm: @zzbot r+


Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks broke.


Comments from Reviewable

zzbot commented 7 years ago

:pushpin: Commit bfba349 has been approved by vheon

zzbot commented 7 years ago

:hourglass: Testing commit bfba349aa0400e8262ca6f8d88c64055c1cff3b9 with merge a9ae0beb128d37f9448aba90904e8f82cf7b151b...

zzbot commented 7 years ago

:sunny: Test successful - status-appveyor, status-travis Approved by: vheon Pushing a9ae0beb128d37f9448aba90904e8f82cf7b151b to master...