GNOME Bugzilla – Bug 686006
Some gvfs-test improvements
Last modified: 2012-11-29 14:55:15 UTC
Thanks Alex for landing the gvfs-test bits yesterday. Nice work on the "run out fo the build tree" bits, this is running very well! You mentioned that some tests are failing for you. For me they both work with running against the system packages (tests/gvfs-test) as well as against the local git master build (make -C test test), so I figure I made some Debian/Ubuntu specific assumptions somewhere. Can you please copy&paste the full output of a test run, so that we can make all tests succeed on Fedora as well? I took the liberty to add the missing author/copyright/license header (http://git.gnome.org/browse/gvfs/commit/?id=684a4961). I have some other refinements, I'll attach them here as patches as we didn't yet talk about how much you want me to commit test suite fixes directly.
Created attachment 226308 [details] [review] gvfs-test: Skip Dav tests if Apache httpd is not installed This is more friendly for eventually running this through "make check" when not all test dependencies are available.
Created attachment 226309 [details] [review] test/run-in-tree.sh: Search session.conf in test directory Allow users to call test/run-in-tree.sh from any directory, so that you can e. g. run "test/run-in-tree.sh gvfs-mount -li". Without this, run-in-tree.sh only finds its session.conf when running this in the tests/ directory. Out of interest, why did you take this approach instead of just calling dbus-launch?
Created attachment 226310 [details] [review] Ship test/run-in-tree.sh in tarballs
Review of attachment 226308 [details] [review]: ack
Review of attachment 226309 [details] [review]: Yeah, this looks good. Needs rebase as i fixed similar issue by using `pwd` instead which is not right.
Review of attachment 226310 [details] [review]: already in master
Comment on attachment 226309 [details] [review] test/run-in-tree.sh: Search session.conf in test directory Ah, you already fixed that in trunk yourself now.
Comment on attachment 226308 [details] [review] gvfs-test: Skip Dav tests if Apache httpd is not installed Pushed.
> Out of interest, why did you take this approach instead of just calling > dbus-launch? I could not get dbus-launch to let me specify a custom config file, which meant i couldn't easily set up a custom .service file search dir. This seems much easier than messing with XDG dirs that have to have a specific directory hieararchy.
Comment on attachment 226309 [details] [review] test/run-in-tree.sh: Search session.conf in test directory Whoops, the `pwd` change did not actually fix running the script from other directories. I rebased it to current master and pushed.
Notes for myself, summary/TODO from IRC discussion with Alex: gvfs-test: (1) add "make installcheck" (2) ship static test .iso to drop genisoimage dependency (3) investigate whether smbd can be run as user on a different port (and drop usershare parts) gvfs-testbed: (1) pkill --signal FOO → pkill -FOO (2) self-contained SSL config, and ship static cert instead of always generating a new one (3) if above (3) doesn't work, run under unshare -n and run smb with full config in testbed, not touching the system one
Created attachment 226324 [details] [review] gvfs-test: Integrate into "make installcheck" This runs the tests against the system instead of the build tree, for system integration/package tests.
Created attachment 226329 [details] [review] Add test .iso files, drop genisoimage dependency We discussed it on IRC, but I'm not sure whether you prefer this. This replaces the genisoimage calls with two static test files, which drops the genisoimage dependency and thus makes the tests a bit more robust. NB that this needs the latest version of http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/quantal/gvfs/quantal/view/head:/debian/tests/gvfs-testbed as this now has to copy the test files into the sandbox.
Review of attachment 226324 [details] [review]: Ack
Review of attachment 226329 [details] [review]: I don't think this is right. genisoimage is not a heavy dependency, and dynamically generating the isos makes it much more dynamic as we add more tests. Is there any particular reason not to use genisoimage?
Created attachment 226440 [details] [review] gvfs-test: Ship static SSL certificate, drop openssl dependency Stop generating an SSL certificate on the fly, it's too brittle across different distributions and unnecessary overhead. Instead, ship a static test SSL certificate.
Comment on attachment 226329 [details] [review] Add test .iso files, drop genisoimage dependency Fine for me; I had the impression you preferred static files, but it seems that was a misunderstanding. Let's drop that patch then.
Created attachment 226441 [details] [review] gvfs-test: Ship static SSL certificate, drop openssl dependency Updated patch to account for the dropped genisoimage patch, which defined my_dir.
Created attachment 226445 [details] [review] gvfs-test: Use local sshdh configuration and host keys Stop relying on the system sshd configuration and calling sshd-keygen on the fly, as this is not reliable across distributions. Just ship a local test host key and generate a complete (but minimal) sshd config on the fly.
Created attachment 226446 [details] [review] gvfs-test: Stop using /var/log/sshd.log Just read sshd's output directly from the process' stderr pipe. With that we can stop relying on that part of gvfs-testbed and get the tests closer to being able to run without gvfs-testbed.
Created attachment 226447 [details] [review] gvfs-test: Support running Sftp without gvfs-testbed We absolutely need to avoid touching ~/.ssh if we don't run under gvfs-testbed: - Only generate ~/.ssh and an SSH key if running under gvfs-testbed; otherwise require that an SSH key already exists. - Tell our local sshd to look for authorized_keys in $XDG_RUNTIME_DIR instead of ~/.ssh/authorized_keys, as we need to modify it for the tests. Make two adjustments to the test which previously assumed English strings (in gvfs-testbed gvfs runs under no locale). Drop depending on $SSHD provided by gvfs-testbed and instead determine the sshd path with "which", requiring that it is in $PATH. This goes together with the simplifications in the current testbed script: http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/quantal/gvfs/quantal/view/head:/debian/tests/gvfs-testbed I'd appreciate if you could test this on Fedora as well (simple "make check" should succeed"), just to ensure that the local ssh server config and assumptions like "sshd is in $PATH" are true there.
Comment on attachment 226445 [details] [review] gvfs-test: Use local sshdh configuration and host keys I just noticed that git doesn't store file permissions, so test/files/ssh_host_rsa_key ends up as 644 when applying this. sshd complains about this, so we need to handle this differently.
Comment on attachment 226447 [details] [review] gvfs-test: Support running Sftp without gvfs-testbed - Always assume that ~/.ssh key exists, let gvfs-testbed create it - Gracefully handle unset XDG_USER_DIRS - error message check: only do this when LC_MESSAGES is en_* or C, instead of checking for in_testbed
Created attachment 226450 [details] [review] gvfs-test: Use local sshd configuration and host keys Updated patch to work around git's inability to track/set file permissions, so we need to copy and chmod the host file. Also fix the typo in the short description.
Created attachment 226451 [details] [review] gvfs-test: Stop using /var/log/sshd.log No functional changes, just a clean rebase against the previous updated patch.
Created attachment 226452 [details] [review] gvfs-test: Support running Sftp without gvfs-testbed Updated patch which now handles missing XDG_RUNTIME_DIR, non-English locales properly, and stops making assumptions about running under gvfs-testbed.
Review of attachment 226451 [details] [review]: ack
Review of attachment 226452 [details] [review]: ack
Created attachment 226533 [details] [review] gvfs-test: Drop usershares We cannot rely on availability of usershares on all distros, so simplify the tests to expect that gvfs-testbed sets up the "myfiles" share by itself and then just use this. As the same share now allows both guest and authenticated access, call gvfs-mount with an "user@" prefix in the URI to tell it that we want authenticated access, and adjust wait_for_gvfs_mount_user_prompt() to also trigger on a Domain prompt (as gvfs-mount now does not ask for the user name any more).
Created attachment 226534 [details] [review] gvfs-test: Robustify Smb.test_anonymous gvfs-mount may hang indefinitely if it unexpectedly asks for credentials when trying to do an anoymous mount. Time out after 5 seconds instead.
Created attachment 226535 [details] [review] gvfs-test: Robustify Smb.test_anonymous Eek, patch was out of date, sorry.
Created attachment 226546 [details] [review] Support running a subset of tests with make check With this you can run e. g. make check TEST_NAMES="Smb Dav.test_http_auth" to only run a subset of tests. Same for "make installcheck".
Created attachment 226547 [details] [review] gvfs-test: Make Smb tests run without gvfs-testbed As discussed on IRC, this allows the Smb tests to run under "make check". This requires the $LIBSMB_PROG hack. Note that this currently has a "FIXME" which is solved in the followup patch. I just didn't want to conflate the two changes into one commit.
Created attachment 226548 [details] [review] gvfs-test: Split "myfiles" share into public and private Clean up tests to work consistently under gvfs-testbed with "real" smbd as well as smbd running as user.
Created attachment 226549 [details] [review] gvfs-test: Fix gvfs-mount waiting This fixes bugs in the gvfs-mount polling.
I documented the Samba approach here, as there's nothing on the interwebs about this so far: http://www.piware.de/2012/10/running-a-samba-server-as-normal-user-for-testing/
Attachment 226546 [details] pushed as c144ca6 - Support running a subset of tests with make check Attachment 226547 [details] pushed as 996f051 - gvfs-test: Make Smb tests run without gvfs-testbed Attachment 226548 [details] pushed as fb3f5e0 - gvfs-test: Split "myfiles" share into public and private Attachment 226549 [details] pushed as fbc059e - gvfs-test: Fix gvfs-mount waiting
Notes for myself: Sftp.test_unknown_host fails with http://www.fpaste.org/7cLj/ for Alex: ====================================================================== FAIL: test_unknown_host (__main__.Sftp) sftp:// with RSA authentication for unknown host ---------------------------------------------------------------------- Traceback (most recent call last):
+ Trace 231053
self.assertNotEqual(code, 0)
This is client-configurable behaviour; it would be better if the test would explicitly verify that gvfs-mount asks for confirmation, and check the result based on the answer.
Created attachment 229629 [details] [review] gvfs-test: Add tests using the introspected Gio API This patch adds some tests that exercise the Gio API. The existing tests all use the command line tools. Please note that it currently has a large timeout of 30 seconds for the mount operation. The biggest annoyance of those tests is that they take effing long when they run under a temporary session D-BUS instead of the real one. When I just run "test/gvfs-test Ftp" (thus test against the installed gvfs package) or I comment out the "set up private D-BUS" part in test/run-in-tree.sh and run TEST_NAMES=Ftp PYTHON=python3 jhbuild run make check then all four Ftp tests take some 12 seconds. However, when leaving the private D-BUS bits, it takes over 2 minutes (!). The two anonymous tests (CLI and API) work as fast as before, but each of the authenticated ones takes a minute. This doesn't seem to be a bug in the tests themselves. When running gvfs-mount, it takes that long for wait_for_gvfs_mount_user_prompt() to get any output on stdout (the first password prompt); when using the API, it takes some 15 seconds until mount_enclosing_volume()'s GMountOperation sends the ask_password signal (and the test call mount 4 times). So this sounds like gvfs is expecting something to be on the session bus which isn't there on the private instance and waits until the D-BUS timeout until it proceeds. Something gnome-keyring-ish perhaps?
For the record, interactive reproducer: $ twistd -n ftp -p 2121 -r /tmp/ --auth 'memory:testuser:pwd1' $ jhbuild run ./run-in-tree.sh gvfs-mount ftp://localhost:2121 (the latter command in the tests/ directory)
Created attachment 229634 [details] [review] tests/run-in-tree.sh: Disable service activation for local D-BUS Indeed, when I run /run-in-tree.sh bash, and in that subshell start dbus-monitor and the gvfs-mount command, I see these four method calls: ------------- 8< ------------------ method call sender=:1.7 -> dest=org.freedesktop.DBus serial=6 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=AddMatch string "type='signal',sender='org.freedesktop.secrets',interface='org.freedesktop.DBus.Properties',member='PropertiesChanged',path='/org/freedesktop/secrets',arg0='org.freedesktop.Secret.Service'" method call sender=:1.7 -> dest=org.freedesktop.DBus serial=7 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=AddMatch string "type='signal',sender='org.freedesktop.secrets',interface='org.freedesktop.Secret.Service',path='/org/freedesktop/secrets'" method call sender=:1.7 -> dest=org.freedesktop.DBus serial=8 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=AddMatch string "type='signal',sender='org.freedesktop.DBus',interface='org.freedesktop.DBus',member='NameOwnerChanged',path='/org/freedesktop/DBus',arg0='org.freedesktop.secrets'" method call sender=:1.7 -> dest=org.freedesktop.DBus serial=9 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=StartServiceByName string "org.freedesktop.secrets" uint32 0 ------------- 8< ------------------ then 15 seconds of pause. There is no "method return" there, so these are timing out. After the pause, I see ------------- 8< --------------- method call sender=:1.11 -> dest=org.freedesktop.DBus serial=10 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=RemoveMatch string "type='signal',sender='org.freedesktop.DBus',interface='org.freedesktop.DBus',member='NameOwnerChanged',path='/org/freedesktop/DBus',arg0='org.freedesktop.secrets'" method call sender=:1.11 -> dest=org.freedesktop.DBus serial=11 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=RemoveMatch string "type='signal',sender='org.freedesktop.secrets',interface='org.freedesktop.DBus.Properties',member='PropertiesChanged',path='/org/freedesktop/secrets',arg0='org.freedesktop.Secret.Service'" method call sender=:1.11 -> dest=org.freedesktop.DBus serial=12 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=RemoveMatch string "type='signal',sender='org.freedesktop.secrets',interface='org.freedesktop.Secret.Service',path='/org/freedesktop/secrets'" method call sender=:1.11 -> dest=:1.9 serial=13 path=/org/gtk/gvfs/mountop/0; interface=org.gtk.vfs.MountOperation; member=AskPassword string "Bitte Passwort für localhost:2121 eingeben" string "" string "" uint32 27 ------------- 8< --------------- The long timeouts are gone if I drop <standard_session_servicedirs /> from test/session.conf.in, i. e. I disallow D-BUS activation of extra services such as gnome-keyring. We actually don't want g-keyring during the tests to avoid potentially meddling with the user's real keyrings. We might need D-BUS activated services for other tests in the future, but right now we don't. I currently get failures with the Dav tests, but these seem to be independent from D-BUS service activation. I'm debugging those now.
Created attachment 229706 [details] [review] gvfs-test: Skip Dav tests if http is disabled Ah, I know why the Dav tests fail, current jhbuild has modulesets/gnome-suites-core-3.8.modules: <autotools id="gvfs" autogenargs="--disable-http"> This patch detects if dav support is enable, and skips the Dav tests if not. Running against the system installed gvfs still runs them though; I'd really expect distro packages to pick a working combination of libsoup and gvfs.
Review of attachment 229629 [details] [review]: looks good to me.
Review of attachment 229634 [details] [review]: Some gvfs backends might rely on dbus activation of some service though? Although i guess in that case we can manually start it...
Review of attachment 229706 [details] [review]: ack
Thanks for the review! The remaining TODO here as far as I can see is the underdefined Sftp.test_unknown_host test from https://bugzilla.gnome.org/show_bug.cgi?id=686006#c38 . Keeping open for that one.
(In reply to comment #38) > Notes for myself: > > Sftp.test_unknown_host fails with http://www.fpaste.org/7cLj/ for Alex: > > ====================================================================== > FAIL: test_unknown_host (__main__.Sftp) > sftp:// with RSA authentication for unknown host > ---------------------------------------------------------------------- > Traceback (most recent call last): For the record, this can be reproduced with setting "StrictHostKeyChecking no" in ~/.ssh/config.
Created attachment 230021 [details] [review] Skip Sftp.test_unknown_host when not in gvfs-testbed This gets rid of the exfail on the "unknown host" ssh test. I don't see a way how to reliably exercise this without gvfs-testbed; if someone has a better idea, I'm all ears (ssh doesn't respect $HOME, $XDG_*, etc., and temporarily changing the user's files is not something I want to do). I think this is the last outstanding issue here. After this I propose that we close this bug as it has grown too large already.
Martin, do you have a test for trash:// ? Would be great to have one to exercise the ::orig-path and ::deletion-date attributes and the helpers to get to those in GIO.
(In reply to comment #49) > Martin, do you have a test for trash:// ? Would be great to have one to > exercise the ::orig-path and ::deletion-date attributes and the helpers to get > to those in GIO. No, we don't yet have those yet, but that's a good idea! I'll create some. This just needs some rearrangements before: we need to launch the local d-bus from within the test suite instead of from test/run-in-tree.sh, so that we can inject a temporary $XDG_DATA_HOME.
Created attachment 230098 [details] [review] Move temporary D-BUS setup from run-in-tree.sh This is a prerequisite for test trash:// and potentially other things in the future. It also makes debugging a lot nicer, as we can capture and display the server-side debugging output. Full rationale is in the commit log.
Created attachment 230099 [details] [review] Add tests for trash:// With the previous patch we can now write tests for trash://. This adds three.
Created attachment 230102 [details] [review] Move temporary D-BUS setup from run-in-tree.sh Fixed failure/error comparison.
Created attachment 230158 [details] [review] Move temporary D-BUS setup from run-in-tree.sh I realized that we can now also put the $LIBSMB_PROG hack at the right place in gvfs-test, no need to inject it from the Makefile. This will do the right thing when calling gvfs-test directly, not through make. Also, we should not set up our own XDG dirs and dbus daemon when running under gvfs-testbed.
Created attachment 230159 [details] [review] Add tests for trash:// trash:// tests, rebased to apply cleanly on top of changed "D-BUS setup" patch.
I pushed the last three patches, with adjusting the trash:// one to also work if /tmp/ is a mounted tmpfs.