-
-
Notifications
You must be signed in to change notification settings - Fork 526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Skip alwayscopy tests for SuSE #753
Conversation
Codecov Report
@@ Coverage Diff @@
## master #753 +/- ##
==========================================
+ Coverage 94.8% 94.84% +0.04%
==========================================
Files 11 11
Lines 2386 2386
==========================================
+ Hits 2262 2263 +1
+ Misses 124 123 -1
Continue to review full report at Codecov.
|
tests/test_z_cmdline.py
Outdated
@@ -635,7 +635,11 @@ def _alwayscopy_not_supported(): | |||
# see: https://github.com/pypa/virtualenv/issues/565 | |||
if hasattr(platform, 'linux_distribution'): | |||
_dist = platform.linux_distribution(full_distribution_name=False) | |||
if _dist[0] == 'centos' and _dist[1][0] == '7': | |||
(name, version, arch) = _dist | |||
if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this formatting looks akward, maybe instead use the any
with two args, one per line 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice trick !
tests/test_z_cmdline.py
Outdated
@@ -635,7 +635,11 @@ def _alwayscopy_not_supported(): | |||
# see: https://github.com/pypa/virtualenv/issues/565 | |||
if hasattr(platform, 'linux_distribution'): | |||
_dist = platform.linux_distribution(full_distribution_name=False) | |||
if _dist[0] == 'centos' and _dist[1][0] == '7': | |||
(name, version, arch) = _dist | |||
if any(( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what I meant
if any(( name == 'centos' and version[0] == '7',
name == 'SuSE' and arch == 'x86_64' )):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current state is closer to this pep8 recommendation:
FILES = [
'setup.cfg',
'tox.ini',
]
This makes suppressions very clear to read in the logs. (1 suppression: 1 line).
But just confirm your preference and I'll update accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to disagree with that, so I would go to change it to be more readable 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, then this is a matter of taste ;-)
So your code was copy/pasted, but failed fix-lint
. Had to remove few spaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as noted above
This is a fix to issue #752.
Now
tox
succeeds (with python versions: py27, py34, py36).a priori the i686 arch should be able to run, so skip
test_alwayscopy
only for the x86-64 arch.