Skip to content

Conversation

@The-Compiler
Copy link
Member

I swear this is the last one 🙈 Take your time to review things though, I'm not in a hurry!

Needed this for some tests in qutebrowser recently, and it seems simple enough to add it.

@coveralls
Copy link

coveralls commented Sep 15, 2018

Coverage Status

Coverage decreased (-2.7%) to 96.408% when pulling a2af859 on The-Compiler:assert-not-emitted-wait into 0a19255 on pytest-dev:master.

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just need to fix the linting errors and we can merge this. 😁

@The-Compiler
Copy link
Member Author

Whoops, accidentally had a ; at the end of the line in the docs 😆 That's what I get for writing JavaScript, C++ and Python on the same day!

@The-Compiler
Copy link
Member Author

Thinking about this some more, I noticed a QEventLoop is spin up every time assertNotEmitted is used now (with the default timeout=0), which seems weird.

I've opted to just not spinning up an event loop with qtbot.waitSignal(..., timeout=0) at all, see the last commit - what do you think?

@nicoddemus
Copy link
Member

I've opted to just not spinning up an event loop with qtbot.waitSignal(..., timeout=0) at all, see the last commit - what do you think?

Seems reasonable, thanks!

@nicoddemus nicoddemus merged commit 488496d into pytest-dev:master Sep 22, 2018
@nicoddemus
Copy link
Member

(and sorry for the delay!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants