Use pytest #30

xZise · 2016-10-20T18:14:19Z

This rewrites the existing tests to be used with pytest which produces a more helpful output when an assertion failed. It also allows to have multiple separate tests in one file but separated in functions.

It also adds some tests when using multiple interfaces in one class and it checks if the results are actually valid instead of just printing them out.

Instead of doing the tests manually use a third party library, like `pytest` to make it easier to do and verify tests.

The tests requiring multiple threads (one for publishing the interfaces and one for using them) are largely rewritten to work with pytest. It doesn't support assertions which were not asserted in the main thread. So the threads need to either report the failed assertion or the actual result and that is asserted in the test itself. It also removes any modification of global variables outside the global scope, because that would make it not possible to run multiple tests. Instead there is a new Thread class which automatically quits the main loop after all tests are finished. If the main loop is quit, the main thread can get the status from the threads to check what the result was. If the thread should hang for any reason, it'll timeout after 100 ms and report it as a failure. Also add more tests for the class using multiple interfaces, by using only a specific interface. The test script itself is rewritten to call pytest only once.

LEW21

Seems like a good idea, as pytest.raises context manager makes the tests much more readable.

However, I'm worried that the threading machinery might produce more problems than it's trying to solve. Especially when I merge the greenglib branch - 833dcd8 - which adds support for greenlets and gevent.

Also, as a comment says, it might lose information about where exactly an exception was raised if it was raised in a thread.

LEW21 · 2016-10-21T13:53:48Z

pydbus/tests/context.py

-	assert(False)
-except RuntimeError:
-	pass
+def test_remove_dbus():


This should be called test_bus_closure - as it tests if connected buses are automatically closed when going outside of the with statement.

LEW21 · 2016-10-21T13:57:15Z

pydbus/tests/context.py


-with bus.request_name("net.lew21.Test"):
-	pass
+def test_use_exited_bus():


This should be called test_standard_bus_object_preservation, or something like that, in line with the comment "SessionBus() and SystemBus() are not closed automatically, so this should work"

LEW21 · 2016-10-21T14:21:09Z

pydbus/tests/util.py

+	def run(self):
+		with self._lock:
+			while not self.loop.is_running:
+				pass


Won't this make us use 100% CPU? That's a no-go.

The reason I added this statement is that if the thread is too fast and tries to run before the loop is actually activated it reacted strangely. Maybe this isn't actually an issue, but if it is the only other reasonable implementation would be to wait a bit (similar to how TimeLock waits a bit).

LEW21 · 2016-10-21T14:28:46Z

pydbus/tests/util.py

+			if self._lock.acquire(False):
+				return True
+			else:
+				time.sleep(0.001)


Meh, busy waiting is not a good idea.

Not sure what you mean here? If the lock cannot be acquired it should wait a bit and retest. There is no way around to either sleep a bit or not at all. It's not like the thread is going to do anything else.

xZise · 2016-10-22T10:29:06Z

To be honest I don't really know how your greenlet implementation is going to work so I can't really say what needs to be changed.

But mainly the advantage of using pytest is that it provides more information and doesn't stop other tests. When I was running my tests regarding #29 yesterday it would only run until the first assertion happened. And even then it wouldn't tell me what went wrong.

And maybe there is actually a way to say where the problematic assertion happened.

xZise · 2016-10-22T14:02:06Z

Okay I have found a way to actually show where the assertion happened, unfortunately the Python 2 code cannot be parsed in Python 3 so I need to find a way round that.

xZise · 2016-10-25T11:42:13Z

Okay I think I've resolved the issue that it didn't show where the assertion happened in both Python versions. Additionally I do think that at some point your repository should use a testing suite. At the moment if you just use assertions it does tell you only that they failed but not why (if there was a comparison what the separate values for example).

So I'm wondering if I should separate the second commit which adds the threading stuff? Because a patch like #29 would benefit from that as it doesn't use threads and thus could be written to be used by pytest. The only downside (I think) is, when with the introduction of greenlets it becomes impossible to use pytest, that the patch might need to be reverted if there is more suitable library.

But I have to say it is cumbersome to run tests because you only get to know that it failed but not what happened (unless you manually print it out). If you are fine with separating the commits please tell me which commit this pull request should contain (aka the first or second). There is also the gnome_music test. It seems to me that this could be easily converted but you can say if I should it include in either of the commits or use a separate commit (but then maybe included in either of the pull requests). One feature of pytest (and others) is to skip unsuitable tests. For example I don't have GNOME music installed so it should skip the test on my machine. At the moment it'd throw an exception and be classified as a failure.

LEW21 · 2016-10-25T12:32:35Z

I've never had any problems with checking why the test has failed. There is a traceback, and the line with assert is included. Of course assert(False)s aren't too readable, but it looks like pytest.raises is usable even when without running the tests with the pytest command:

>>> import pytest
>>> with pytest.raises(AttributeError):
...   pass
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File "/usr/lib/python3.5/site-packages/_pytest/python.py", line 1217, in __exit__
    pytest.fail(self.message)
  File "/usr/lib/python3.5/site-packages/_pytest/runner.py", line 537, in fail
    raise Failed(msg=msg, pytrace=pytrace)
Failed: DID NOT RAISE <class 'AttributeError'>
>>> with pytest.raises(AttributeError):
...   raise AttributeError()
... 
>>>

LEW21 · 2016-10-25T12:40:41Z

And BTW, the GNOME Music test is not really useful for testing pydbus. I've made it when I was porting GNOME Music to pydbus (however, they've chosen to go straight for GDBus). It isn't 100% automated, depends on external circumstances (for example the user has to have some songs in his GNOME Music collection), it's slow, affects the user, and it's impossible to use on Travis - while I'm almost exclusively using Travis to run the test collection.

It's there, because it somewhat works, but it's not a good idea to run it automatically.

xZise · 2016-10-25T14:14:25Z

Here is an example what is shown when the assertion failed. To force it I used this branch and modified pydbus/tests/identifier.py:

>       assert filter_identifier(input) == output
E    assert 'abc' == 'abcd'
E      - abc
E      + abcd
E      ?    +

pydbus/tests/identifier.py:13: AssertionError

If I don't use pytest I get the following:

$ python -c "import pydbus.tests.identifier as I; I.test_filter_identifier('abc', 'abcd')"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "pydbus/tests/identifier.py", line 13, in test_filter_identifier
    assert filter_identifier(input) == output
AssertionError

Apart from that it does only stops the test function itself after an assertion failed. Other test functions will still be run. The other tests may fail as well, but if they don't depend on each other (and they shouldn't, otherwise they should be in the same test function) it may tell more about the actual issue. If you look in the tests of #29 you'll see that you can basically run each test independently.

xZise added 2 commits October 20, 2016 19:59

Use pytest module

f1638ef

Instead of doing the tests manually use a third party library, like `pytest` to make it easier to do and verify tests.

LEW21 requested changes Oct 21, 2016

View reviewed changes

This was referenced Dec 9, 2016

Refactor tests runner script to use dbus-launch instead of dbus-daemon #38

Merged

How to run tests ? #40

Closed

eli-schwartz mentioned this pull request Jan 20, 2020

Support running tests via python setup.py test #85

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use pytest #30

Use pytest #30

xZise commented Oct 20, 2016

LEW21 left a comment

LEW21 Oct 21, 2016

LEW21 Oct 21, 2016

LEW21 Oct 21, 2016

xZise Oct 22, 2016

LEW21 Oct 21, 2016

xZise Oct 22, 2016

xZise commented Oct 22, 2016

xZise commented Oct 22, 2016

xZise commented Oct 25, 2016

LEW21 commented Oct 25, 2016

LEW21 commented Oct 25, 2016

xZise commented Oct 25, 2016

Use pytest #30

Are you sure you want to change the base?

Use pytest #30

Conversation

xZise commented Oct 20, 2016

LEW21 left a comment

Choose a reason for hiding this comment

LEW21 Oct 21, 2016

Choose a reason for hiding this comment

LEW21 Oct 21, 2016

Choose a reason for hiding this comment

LEW21 Oct 21, 2016

Choose a reason for hiding this comment

xZise Oct 22, 2016

Choose a reason for hiding this comment

LEW21 Oct 21, 2016

Choose a reason for hiding this comment

xZise Oct 22, 2016

Choose a reason for hiding this comment

xZise commented Oct 22, 2016

xZise commented Oct 22, 2016

xZise commented Oct 25, 2016

LEW21 commented Oct 25, 2016

LEW21 commented Oct 25, 2016

xZise commented Oct 25, 2016