Skip to content
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

Merge from master to features #1705

Merged
merged 34 commits into from Jul 8, 2016

Conversation

@RonnyPfannschmidt
Copy link
Member

RonnyPfannschmidt commented Jul 6, 2016

new pr

The-Compiler and others added 30 commits Jun 22, 2016
issue1625, rename getfuncargvalue to getfixturevalue
Fixes #1503 no longer collapse false explanations
I have came across this when noticing that universal-ctags fails to parse
this correctly (universal-ctags/ctags#997).
This was added in b2d66b9 but is a bad
idea. When a plugin can't be imported, commandline options (optionally
set in pytest.ini) could be undefined, which means pytest bails out
much earlier before showing the warning, which is hard to debug.

Fixes #1479, also see #1307 and #1497
Combining multiple imports the "import pdb" imports the pytest module
as opposed to the python debugger.
Don't ignore ImportError when importing setuptools plugins
Update website wrt indiegogo campaign
Simplify Argument.__repr__
Add changelog entry for #1564
Rename the default plugin "pdb" into "debugging"
Even though catch is a Python keyword, 'catched' just looks terrible in text.
If the text was supposed to reference the keyword, then 'catched' should be changed to "'catch'ed".
catched -> caught
I wouldn't have even attempted what I did to cause #1688 if this had
been there.
Document the interaction of autouse scopes
Add changelog to requirements for pytest-dev plugins
Some of our builds have been timing out (over 1 hour),
on AppVeyor
The-Compiler and others added 3 commits Jul 5, 2016
Split AppVeyor test runs in multiple jobs to avoid timeout issues
Fix links and removed 404 links from talks.rst
@coveralls
Copy link

coveralls commented Jul 6, 2016

Coverage Status

Coverage increased (+0.05%) to 92.143% when pulling b9a91dc on RonnyPfannschmidt:merge-master into f31c31a on pytest-dev:features.

@@ -187,6 +210,9 @@
.. _@Vogtinator: https://github.com/Vogtinator
.. _@blueyed: https://github.com/blueyed
.. _@fengxx: https://github.com/fengxx
.. _@bagerard: https://github.com/bagerard
.. _@davehunt: https://github.com/davehunt

This comment has been minimized.

Copy link
@The-Compiler

The-Compiler Jul 6, 2016

Member
INFO CHANGELOG.rst:242 Duplicate explicit target name: "@davehunt".

(he's already on line 216/242)

@coveralls
Copy link

coveralls commented Jul 6, 2016

Coverage Status

Coverage increased (+0.05%) to 92.143% when pulling dad6aa8 on RonnyPfannschmidt:merge-master into f31c31a on pytest-dev:features.

@nicoddemus
Copy link
Member

nicoddemus commented Jul 6, 2016

Hmm it failed on Windows:

_________________ ERROR collecting testing/python/metafunc.py _________________
_pytest\python.py:374: in fget
    return self._obj
   AttributeError: 'Module' object has no attribute '_obj'
During handling of the above exception, another exception occurred:
...
.tox\py35-xdist\lib\site-packages\hypothesis\internal\charmap.py:77: in categories
    cm = charmap()
.tox\py35-xdist\lib\site-packages\hypothesis\internal\charmap.py:63: in charmap
    os.rename(tmpfile, f)
FileExistsError: [WinError 183] Cannot create a file when that file already exists: 'C:\\projects\\pytest\\.hypothesis\\tmp\\tmp9lzkh68w' -> 'C:\\projects\\pytest\\.hypothesis\\unicodedata\\8.0.0\\charmap.pickle.gz'
_________________ ERROR collecting testing/python/metafunc.py _________________

The first error ('Module' object has no attribute '_obj') might be related to this comment, although I find the error about hypothisis suspicious, specially because it happend on an xdist environment...

@RonnyPfannschmidt
Copy link
Member Author

RonnyPfannschmidt commented Jul 7, 2016

at second glance it seems like a hypothesis bug @DRMacIver might know more

@flub
Copy link
Member

flub commented Jul 7, 2016

So can we merge this anyway? But every single PR will show this issue? How wise is it to make all PRs dependent on an external project?

@The-Compiler
Copy link
Member

The-Compiler commented Jul 7, 2016

To be honest I'd rather wait a day or two to see what @DRMacIver has to say about it - and if it really turns out to be a hypothesis issue and we don't find a quick fix, just blacklist the hypothesis tests for now, merge this, and then investigate afterwards.

How wise is it to make all PRs dependent on an external project?

Do you mean in general, or if we'd merge this?

@nicoddemus
Copy link
Member

nicoddemus commented Jul 7, 2016

To be honest I'd rather wait a day or two to see what @DRMacIver has to say about it - and if it really turns out to be a hypothesis issue and we don't find a quick fix, just blacklist the hypothesis tests for now, merge this, and then investigate afterwards.

This merge is long overdue now, so I would rather do it now rather than wait a few more days. I propose we report the issue to hypothesis repository and xfail the test which depends on Hypothesis, but only when running on xdist:

xdist = request.config.pluginmanager.getplugin('xdist')
if xdist:
    pytest.xfail(reason='bad interaction with xdist, waiting on HypothesisWorks/hypothesis-python/#X ')

How wise is it to make all PRs dependent on an external project?

All PRs already depend on a bunch of external projects: nose, mock or even colorama, just to name a few. Hypothesis is no different IMO. Bugs in external projects are part of life. 😁

@flub
Copy link
Member

flub commented Jul 8, 2016

@nicoddemus It is a collection error though, maybe we should be adding .hypothesis to norecursedirs in tox.ini? It's more a stab in the dark then really understanding the issue tough.

Sure about the external dep, it's unavoidable I guess. It's just a bit annoying that this seemingly unrelated thing is holding up what should be a routine merge.

@The-Compiler
Copy link
Member

The-Compiler commented Jul 8, 2016

It is a collection error though, maybe we should be adding .hypothesis to norecursedirs in tox.ini? It's more a stab in the dark then really understanding the issue tough.

It happens at collection time because the @hypothesis.given decorator (which is in the stacktrace) gets run then.

@The-Compiler
Copy link
Member

The-Compiler commented Jul 8, 2016

Oh! According to HypothesisWorks/hypothesis#344 a fix is out in Hypothesis 3.4.1 (this is why I suggested waiting, @DRMacIver is incredibly quick with fixing bugs 😆).

Since we don't seem to pin test dependency versions, rerunning the build now.

@nicoddemus
Copy link
Member

nicoddemus commented Jul 8, 2016

@DRMacIver is incredibly quick with fixing bugs

Oh I didn't know that, otherwise I might have suggested the same! 😁

Thanks @DRMacIver!

@nicoddemus
Copy link
Member

nicoddemus commented Jul 8, 2016

I'm hitting that "Merge" button the second I see that AppVeyor build green! 😆

@nicoddemus nicoddemus merged commit 293351c into pytest-dev:features Jul 8, 2016
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@nicoddemus
Copy link
Member

nicoddemus commented Jul 8, 2016

Thanks everyone! 👍

@DRMacIver
Copy link

DRMacIver commented Jul 8, 2016

It happens at collection time because the @hypothesis.given decorator (which is in the stacktrace) gets run then.

Hmm. It's interesting that it errors there though. It's supposed to defer as much as possible to test execution time to speed up file loading and localize errors better.

@RonnyPfannschmidt RonnyPfannschmidt deleted the RonnyPfannschmidt:merge-master branch Aug 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.