Fix version comparison handling. #160

Natim · 2018-11-05T14:52:40Z

Fixes #159

codecov-io · 2018-11-05T15:01:14Z

Codecov Report

Merging #160 into master will increase coverage by 44.53%.
The diff coverage is 33.33%.

@@             Coverage Diff             @@
##           master     #160       +/-   ##
===========================================
+ Coverage   29.42%   73.95%   +44.53%     
===========================================
  Files           4        4               
  Lines         503      503               
  Branches        0       86       +86     
===========================================
+ Hits          148      372      +224     
+ Misses        355      105      -250     
- Partials        0       26       +26

Impacted Files	Coverage Δ
setup.py	`0% <ø> (ø)`	⬆️
pytest_sugar.py	`65.98% <33.33%> (+65.98%)`	⬆️
test_sugar.py	`99.31% <0%> (-0.69%)`	⬇️

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 e8cb804...4e104fd. Read the comment docs.

RonnyPfannschmidt

thats the way to go

Natim · 2018-11-05T15:11:26Z

I think it's missing in tox.ini's testenv.deps as well

If it is present in the install_requires of the setup.py tox will install it.

michael-k · 2018-11-05T15:22:04Z

If it is present in the install_requires of the setup.py tox will install it.

Ah, I see, it's just not updating dependencies. But I guess that's save enough since packaging 14.1 is over 4 years old.

Natim · 2018-11-05T15:40:58Z

For some reason tests might segfault with pypy but it was the case on master before this pull-request.

tox.ini

setup.py

jaraco · 2018-11-05T19:21:37Z

pytest_sugar.py

@@ -398,7 +399,7 @@ def pytest_runtest_logstart(self, nodeid, location):
        # show the module_name & in verbose mode the test name.
        pass

-    if pytest.__version__ >= '3.4':
+    if parse(pytest.__version__) >= parse('3.4'):  # pragma: no cover


I don't like the way the code is written with the "current" version being marked as the deviant behavior. I'd write the code differently:

def pytest_runtest_logfinish(self): """prevent the default implementation to try to show pytest's default progress""" if parse(pytest.__version__) < parse(3.4): # pragma: nocover # On older Pytest, allow default progress del pytest_runtest_logfinish

And then you might even put the if block in a try/except so even if it fails, the more lenient behavior remains.

What would be the harm of including this clause unconditionally, just like its sister method above?

Yes it makes sense, I will do that.

jaraco

Appreciate the work on this - I'm sure it's breaking a lot of builds. My stylistic considerations should not be reason to delay the release of any fix.

blueyed · 2018-11-06T11:22:59Z

.travis.yml

+    - env: TOXENV=py37-pytest310-supported-xdist
+      python: '3.7'
+      sudo: required
+      dist: xenial


Can we use py36 here, since it is easier to setup on Travis?

Do you mean we shouldn't run tests on Python 3.7 ?
I don't understand what you want to say here.

One py37 job is good (which we have).
Here py36-pytest310-supported-xdist could be used.
However, not really important - it just makes the matrix cleaner and the build a bit faster.
Feel free to skip it however.

@michael-k since this is your code, your voice counts.

I was chiming in since he copied my code (edb4fe6).. ;)

setup.py

blueyed · 2018-11-06T14:14:47Z

Builds are failing due to "rerunfailures" (I've cleared the caches and restarted the previous build already): https://travis-ci.org/Frozenball/pytest-sugar/jobs/451271386#L591

blueyed · 2018-11-06T14:17:28Z

We should only install pytest-rerunfailures with pytest >= 3.6 (tox.ini) I guess.

Natim · 2018-11-06T14:18:55Z

We should only install pytest-rerunfailures with pytest >= 3.6 (tox.ini) I guess.

Why is that? However I think this might be out of the scope of this PR

blueyed · 2018-11-06T14:21:50Z

Because it is not pinned, and they have that requirement by now.
I can look into fixing this later.
Also the py37 method you are using now (via deadsnakes repo) is rather slow, too.

blueyed · 2018-11-06T14:23:33Z

..and it appears to have no ssl support: https://travis-ci.org/Frozenball/pytest-sugar/jobs/451390861#L600

blueyed · 2018-11-06T14:25:21Z

I'll push a fixup.

It requires pytest >= 3.6 by now.

blueyed · 2018-11-06T14:29:19Z

.travis.yml

  - TOXENV=py35-pytest30-unsupported-xdist
  - TOXENV=pypy-pytest30-supported-xdist
  - TOXENV=pypy-pytest31-supported-xdist
+  - TOXENV=py37-pytest39-supported-xdist


Will remove that.

blueyed · 2018-11-06T14:30:42Z

tox.ini

          qa

 [testenv]
+usedevelop = True


I guess this was done to get the coverage right?
While it is faster, I think it is better to not use this on CI.

Yes it is so that we use the right file for the coverage.

tox.ini

blueyed · 2018-11-07T09:14:56Z

pytest_sugar.py

+
+
+# On older version of Pytest, allow default progress
+if parse(pytest.__version__) <= parse('3.4'):  # pragma: no cover


Just out of curiosity: why the "# pragma: no cover" here?
Shouldn't it either get tested in some build job, or otherwise show up as not being covered then?

It is covered on some build job but only old version are supposed to cover this.
Do you know if codecov will accumulate all the build coverage?

Yes.. it also uses flags for different builds.

Will address this when rebasing #148.

Thanks for the PR!
I hope we get it released soon.
/cc @Frozenball

michael-k mentioned this pull request Nov 5, 2018

Fix support for pytest >= 3.10 #161

Closed

michael-k and others added 3 commits November 5, 2018 16:00

Fix support for pytest >= 3.10

9986a1a

Regards Teemu#133 Fixes Teemu#159

Fix version comparison handling.

bee857d

Fixes Teemu#159

Add packaging to the list of dependencies.

315ed07

Natim force-pushed the fix-version-comparison branch from 3104d33 to 315ed07 Compare November 5, 2018 15:01

RonnyPfannschmidt approved these changes Nov 5, 2018

View reviewed changes

@michael-k review.

d3d6263

cover pytest_sugar files only.

5f557c2

blueyed reviewed Nov 5, 2018

View reviewed changes

tox.ini Outdated Show resolved Hide resolved

larsoner reviewed Nov 5, 2018

View reviewed changes

setup.py Show resolved Hide resolved

jaraco reviewed Nov 5, 2018

View reviewed changes

jaraco approved these changes Nov 5, 2018

View reviewed changes

Rémy HUBSCHER added 2 commits November 6, 2018 09:58

Improve coverage configuration.

335a298

@jaraco review.

c621bb2

Natim force-pushed the fix-version-comparison branch from cb4d9db to c621bb2 Compare November 6, 2018 09:05

Natim mentioned this pull request Nov 6, 2018

Update pytest to 3.10.0 Kinto/kinto#1861

Closed

blueyed reviewed Nov 6, 2018

View reviewed changes

larsoner mentioned this pull request Nov 6, 2018

MRG: Remove pytest-sugar mne-tools/mne-python#5697

Merged

@blueyed review.

6568ecb

fixup! @blueyed review.

64d77a5

blueyed added 2 commits November 6, 2018 15:27

Only use pytest-rerunfailures with pytest310 job

fb999ce

It requires pytest >= 3.6 by now.

fixup! fixup! @blueyed review.

14573fc

blueyed reviewed Nov 6, 2018

View reviewed changes

Natim commented Nov 6, 2018

View reviewed changes

tox.ini Show resolved Hide resolved

Revert/improve coverage tracking

4e104fd

blueyed merged commit 1d89359 into Teemu:master Nov 7, 2018

blueyed reviewed Nov 7, 2018

View reviewed changes

blueyed mentioned this pull request Nov 7, 2018

[WIP] coverage: configure eager coverage #148

Closed

Natim deleted the fix-version-comparison branch November 7, 2018 09:57

blueyed mentioned this pull request Nov 13, 2018

Use packaging/pkg_resources to check minversion pytest-dev/pytest#4379

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix version comparison handling. #160

Fix version comparison handling. #160

Natim commented Nov 5, 2018

codecov-io commented Nov 5, 2018 •

edited

Loading

RonnyPfannschmidt left a comment

Natim commented Nov 5, 2018 •

edited

Loading

michael-k commented Nov 5, 2018

Natim commented Nov 5, 2018

jaraco Nov 5, 2018 •

edited

Loading

jaraco Nov 5, 2018

Natim Nov 6, 2018

jaraco left a comment

blueyed Nov 6, 2018

Natim Nov 6, 2018

blueyed Nov 6, 2018

Natim Nov 6, 2018

blueyed Nov 6, 2018

blueyed commented Nov 6, 2018

blueyed commented Nov 6, 2018

Natim commented Nov 6, 2018

blueyed commented Nov 6, 2018

blueyed commented Nov 6, 2018

blueyed commented Nov 6, 2018

blueyed Nov 6, 2018

blueyed Nov 6, 2018

Natim Nov 6, 2018

blueyed Nov 7, 2018

Natim Nov 7, 2018

blueyed Nov 7, 2018

blueyed Nov 7, 2018

blueyed Nov 7, 2018



		# On older version of Pytest, allow default progress
		if parse(pytest.__version__) <= parse('3.4'): # pragma: no cover

Fix version comparison handling. #160

Fix version comparison handling. #160

Conversation

Natim commented Nov 5, 2018

codecov-io commented Nov 5, 2018 • edited Loading

Codecov Report

RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

Natim commented Nov 5, 2018 • edited Loading

michael-k commented Nov 5, 2018

Natim commented Nov 5, 2018

jaraco Nov 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaraco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blueyed commented Nov 6, 2018

blueyed commented Nov 6, 2018

Natim commented Nov 6, 2018

blueyed commented Nov 6, 2018

blueyed commented Nov 6, 2018

blueyed commented Nov 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Nov 5, 2018 •

edited

Loading

Natim commented Nov 5, 2018 •

edited

Loading

jaraco Nov 5, 2018 •

edited

Loading