Re: [SyncEvolution] server class + fork/exec dbus-server + command line testing

On Fr, 2012-04-13 at 14:38 +0200, Patrick Ohly wrote: The way how they are captured doesn't allow checking the right order of output via stdout and stderr, because they are stored in separate strings. But I am hard-pressed to suggest a better solution: one could write a custom communicate() method which uses two pipes and copies into one string for each stream and one combined string. But that suffers from race conditions. If output is ready in both streams when the parents starts to collect it, then the information about which output is older is already lost. I'm inclined to forgo testing the stdout/stderr distinction in most tests: use the same pipe for both (so that we can test the order and thus how it really looks to users) in most tests and then write a few tests which check for stdout/stderr usage (and ignore the order). Actually, any test which expects output only via one pipe can use two pipes, because order doesn't matter when one pipe never has data. Below my patch to make the tests work for me: 1. I had to set SYNCEVOLUTION_TEMPLATE_DIR before invoking test-dbus.py, otherwise os.environ["SYNCEVOLUTION_TEMPLATE_DIR"] fails. This really should be rethought. 2. As suspected, the wrong, system syncevo-dbus-server was used because of the dbus-launch. 3. Output checking only gives "normal" results when syncevo-dbus-server is run without SYNCEVOLUTION_DEBUG. @property('debug', False) achieves that (better name welcome). This is just a quick-and-dirty solution, not committed anywhere. Shall I do the base work for a few tests and then let you continue with adding the rest? diff --git a/test/test-dbus.py b/test/test-dbus.py index 262cf16..1c38ed8 100755 --- a/test/test-dbus.py +++ b/test/test-dbus.py @@ -420,7 +420,8 @@ class DBusUtil(Timeout): # always print all debug output directly (no output redirection), # and increase log level - env["SYNCEVOLUTION_DEBUG"] = "1" + if self.getTestProperty("debug", True): + env["SYNCEVOLUTION_DEBUG"] = "1" # can be set by a test to run additional tests on the content # of the D-Bus log @@ -3890,7 +3891,7 @@ def sortConfig(config): return newconfig def runSyncEvolution(argsstr, envstr): - return subprocess.Popen("env `dbus-launch` " + envstr + " sh -c 'trap \"kill $DBUS_SESSION_BUS_PID\" EXIT; syncevolution " + argsstr + "'", shell = True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + return subprocess.Popen("env " + envstr + " syncevolution " + argsstr, shell = True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) def lastLine(string): return string.splitlines(True)[-1] @@ -4009,7 +4010,11 @@ class TestCmdline(unittest.TestCase, DBusUtil): envstr = "SYNCEVOLUTION_TEMPLATE_DIR=" + os.environ["SYNCEVOLUTION_TEMPLATE_DIR"] + " XDG_CONFIG_HOME=" + self.testdir + " HOME=" + self.testdir syncevo = runSyncEvolution(argsstr, envstr) - syncevo.communicate() + result = syncevo.communicate() + print "syncevolution result:", result, syncevo + self.assertEqual(0, syncevo.returncode, result[0] + result[1]) + self.assertEqual("", result[0]) + self.assertEqual("", result[1]) res = sortConfig(scanFiles(root)) res = self.removeRandomUUID(res) res = self.removeSSLCertsPaths(res) @@ -4029,13 +4034,14 @@ class TestCmdline(unittest.TestCase, DBusUtil): shutil.rmtree(peer, True) argsstr = "--configure --sync-property \"deviceId = fixed-devid\" scheduleworld" syncevo = runSyncEvolution(argsstr, envstr) - syncevo.communicate() + result = syncevo.communicate() res = scanFiles(root) res = self.removeSSLCertsPaths(res) expected = ScheduleWorldConfig() expected = sortConfig(expected) self.diffStrings(expected, res) + @property('debug', False) def testSetupScheduleWorld(self): """TestCmdline.testSetupScheduleWorld - TODO: description""" self.doSetupScheduleWorld(False) -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter.