From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Michael Albinus Newsgroups: gmane.emacs.bugs Subject: bug#43252: 27.1; DBus properties lack type hints or overrides Date: Fri, 18 Sep 2020 15:42:45 +0200 Message-ID: <87tuvv8alm.fsf@gmx.de> References: <87v9gqquct.fsf@ccss.com> <87imcqdo38.fsf@gmx.de> <87sgbtqylc.fsf@ccss.com> <87sgbtcvqs.fsf@gmx.de> <87pn6xqtsz.fsf@ccss.com> <87363sl4hs.fsf@gmx.de> <877dt3bnfl.fsf@ccss.com> <878sdj84kn.fsf@gmx.de> <871rjbaq05.fsf@ccss.com> <874ko6979w.fsf@gmx.de> <87v9gm9x9i.fsf@ccss.com> <871rj9k78r.fsf@gmx.de> <87imclwow5.fsf@gmx.de> <87pn6t9rbq.fsf@ccss.com> <87y2lggzvd.fsf@gmx.de> <87h7rzadlo.fsf@ccss.com> <87k0wtrir2.fsf@gmx.de> <87een19x9n.fsf@ccss.com> <87a6xor25b.fsf@gmx.de> <87bli49rem.fsf@ccss.com> <878sd7a99p.fsf@ccss.com> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="18789"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: 43252@debbugs.gnu.org To: Hugh Daschbach Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Sep 18 15:43:42 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kJGft-0004nr-MS for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 18 Sep 2020 15:43:41 +0200 Original-Received: from localhost ([::1]:56668 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kJGfs-00044g-IY for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 18 Sep 2020 09:43:40 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:49988) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kJGfG-00041f-Mw for bug-gnu-emacs@gnu.org; Fri, 18 Sep 2020 09:43:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:58096) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kJGfG-0004ZJ-CP for bug-gnu-emacs@gnu.org; Fri, 18 Sep 2020 09:43:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kJGfG-0003Ng-A8 for bug-gnu-emacs@gnu.org; Fri, 18 Sep 2020 09:43:02 -0400 X-Loop: help-debbugs@gnu.org In-Reply-To: <87v9gqquct.fsf@ccss.com> Resent-From: Michael Albinus Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 18 Sep 2020 13:43:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 43252 X-GNU-PR-Package: emacs Original-Received: via spool by 43252-submit@debbugs.gnu.org id=B43252.160043657712977 (code B ref 43252); Fri, 18 Sep 2020 13:43:02 +0000 Original-Received: (at 43252) by debbugs.gnu.org; 18 Sep 2020 13:42:57 +0000 Original-Received: from localhost ([127.0.0.1]:41407 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kJGfA-0003NE-LA for submit@debbugs.gnu.org; Fri, 18 Sep 2020 09:42:56 -0400 Original-Received: from mout.gmx.net ([212.227.17.21]:60811) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kJGf8-0003Mv-US for 43252@debbugs.gnu.org; Fri, 18 Sep 2020 09:42:55 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1600436566; bh=rxmrp30gZqNwq0FjnbgqpaL/D3KhFMpR9T8AWvrfGuM=; h=X-UI-Sender-Class:From:To:Cc:Subject:References:Date; b=jyiSGIEMc8ESggnw8xgYGK4CZz0LahOtMyFGCgDkJOrCmK8xkWn8QkQsq3EnuggoU utdUUUixOA1KcRWvuAsW2XRnpPddNR39ZO2PnLkcieLQIhTGgHgJDOAX3hplzyg/7z rXtP7ZA/ROuPdkF9kO/cpzw3GO/GxtumSIBM2Bbg= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Original-Received: from gandalf.gmx.de ([178.20.93.248]) by mail.gmx.com (mrgmx105 [212.227.17.168]) with ESMTPSA (Nemesis) id 1Msq6C-1kczPC1f0i-00tDMV; Fri, 18 Sep 2020 15:42:46 +0200 X-Provags-ID: V03:K1:XeuCJxeTxrb9CSUWbTGomjRvz677PVnQ/ortM3NExpKS488M32l L+SPx1Q9fsth4Cl0vU6SkcDVO8wtRNHOo01ibGmbYTL21iMnncGmTt1trsm/A9nez6jsesb whoJdhCdfO86uQi5VActia3bMsfwr05AVkQc+IX5LXcdVupW5Y37vSwEyqXTXrF9qNkQMJ5 qtfc5Iu3myWY0aN9OuYbw== X-UI-Out-Filterresults: notjunk:1;V03:K0:wEBbd3009TA=:yqH8dAi1equM9ZesUvfS63 mRRuvAEHLU95ExLOad3/NnXHJ9d5+T1sCr51bAqIUTCvT1MknXV765GxVJaTmW20zbO944+yK LLawELO/R7sA3yCWjCSlP1aZ7s2dqohCJVY/Ff+KidN/8Mhg4ZsEbxcfgzbsuD4D2N6GTu8j2 EBOTL9k5FyFjiBAuj/uwkt+UhTEAzOUoPzfPGOvxfn8h7YLHmDxl2BB+xQHwyPChvd4ZXC2to yDnI/yM0dVShI2Eg3B3/FK8ZniFOJ13EeSBM0mAifvkzP5VPElY6fjDhfiP+USSau57SaXbxL egiEeKgsnkVzbSUXuZEhUjb4o2F/R0Q1VfWTNaz9hl5JM43QZEwSUqMwusZWa4q9BUVstqs3h tB17IoCf051FTzg5DRcU6E9nYUfShIvCuc28HJH2Pjp3IlKrHGxi0ZuAN11SuTEVJbTLwq1oF FI8pfOd7T8JehRcFlJvG9ePuRDWBpyYxOprz5qWvUTVrvEyddlLnaQkBC5kaxlqOcJalRvCI2 aC6Pt0TiVofeLvqczd7FfnxxKZ2IYzltjgwPcLZyn61BBq1uooiFgDMPsvAf0dj7H7b4fKZLh Z4DeGWanmqf5FbRuoQrPqPzrrsCsTX0gZDJceVq3OYTLCCpK16vqtYZjPz41fQyVykKV41woH gD/r7xRXRCdWgyGJTssMUwiQ3+JpNvMSWvqbfbJvlILm2nQ21zl3K/erSqNVpTDq/L42u5xMJ CDd4N1m7ZPyoaghsOlB/hkVuigx7jfD6zQdGB68RYgMeCrS1x/VRa71oc3Fd6I9JmPZIZfJP X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:188308 Archived-At: Hugh Daschbach writes: Hi Hugh, In general, your tests are very useful. Thanks! Just some comments on your patch. > Add tests that should fail, like setting a property with a type > different from it's type at registration time. As said the other message, this constraint doesn't exist any longer. Registered services might want to control, which properties are set. This could be the type of a property, or a restriction of the value (for example, just a predefined set of strings could be allowed). Maybe, we give these services such a mean? That is, they could add an own handler function in dbus-register-property, which is applied when a org.freedesktop.DBus.Properties.Set method call is handled. WDYT? > +(defun dbus-test06-make-property-test (selector name value expected) I would call it dbus--test-make-property-test. test06 isn't important, and the double slash "--" is an indication that this is an internal function, which shouldn't leave the dbus-tests.el scope. See the other helper functions in the file. > +;; Since we don't expect this helper function and it's caller > +;; `dbus-test06-make-property' to be used outside this file, we don't > +;; bother with `eval-and-compile.' It would be appropriate to wrap > +;; this with `eval-and-compile' if that expectation is misguided. Well, it is uncommon that a function returns a code snippet. I haven't checked, but couldn't you achieve your goal by changing this defun into a defsubst? > +(defmacro dbus-test06-test-property (name value-list) Same comment on name here. I would call it dbus--test-property. > +The argument VALUES is a list of pairs, where each pair > +represents a value form and an expected returned value form. The > +first pair in VALUES is used for `dbus-register-property'. > +Subsequent pairs of the list are tested with > +`dbus-set-property'." The second argument is VALUE-LIST, not VALUES. However, Elisp encourages an argument list like (defmacro dbus-test-test-property (name &rest value-list) This simplifies call conventions, you can call then with several key-value arguments like (dbus--test-property "ByteArray" '((:array :byte 1 :byte 2 :byte 3) . (1 2 3)) '((:array :byte 4 :byte 5 :byte 6) . (4 5 6))) > +(defmacro with-dbus-monitor (buffer &rest body) Such a macro name would poison your Elisp name space. Keep the dbus--test prefix, and name the macro like dbus--test-with-dbus-monitor. > + (unwind-protect > + (progn ,@body) > + (sit-for 0.5) sit-for is problematic, because it would delay the test run by 0.5 seconds, unconditionally. People regard this negative, because the (whole) Emacs test suite shall run fast. A better check might be (with-timeout (1 (dbus--test-timeout-handler)) (while (accept-process-output process 0 nil t))) > + (should > + (equal > + (dbus-register-property > + :session dbus--test-service dbus--test-path > + dbus--test-interface "StringArray" :read > + '(:array "one" "two" :string "three")) > + `((:property :session ,dbus--test-interface "StringArray") > + (,dbus--test-service "/org/gnu/Emacs/TestDBus")))) You might use ,dbus--test-path instead. Here and everywhere else. > + > + (should ; Should this error instead? > + (equal > + (dbus-set-property > + :session dbus--test-service dbus--test-path > + dbus--test-interface "StringArray" > + '(:array "seven" "eight" :string "nine")) > + nil)) Good question. dbus-set-property and dbus-get-property do not propagate D-Bus errors. Maybe we shall change the functions to do so? I've asked this already myself. > + ;; Test mismatched types in array > + > + (should ; Oddly enough, register works, but get fails > + (equal > + (dbus-register-property > + :session dbus--test-service dbus--test-path > + dbus--test-interface "MixedArray" :readwrite > + '(:array > + :object-path "/node00" > + :string "/node01" > + :object-path "/node0/node02")) > + `((:property :session ,dbus--test-interface "MixedArray") > + (,dbus--test-service "/org/gnu/Emacs/TestDBus")))) Hmm, yes. dbus-register-property does not perform a local type check. And honestly, I don't want to do it; I let the D-Bus daemon do the job. > + (should-error > + (equal > + (dbus-get-property > + :session dbus--test-service dbus--test-path > + dbus--test-interface "MixedArray") > + '("/node00" "/node01" "/node0/node02"))) Yes, dbus-get-property is hit by the mismatched types in the :array. Isn't this sufficient? > + (should ; This should error or the next get should fail > + (equal > + (dbus-set-property > + :session dbus--test-service dbus--test-path > + dbus--test-interface "ByteValue" 1024) > + 1024)) No error expected. You haven't given 1024 a type (like :byte), so it is handled as :uint32. > + ;; Test invalid type specification > + > + (should > + (equal > + (dbus-register-property > + :session dbus--test-service dbus--test-path > + dbus--test-interface "InvalidType" :readwrite > + :keyword 128) > + `((:property :session ,dbus--test-interface "InvalidType") > + (,dbus--test-service "/org/gnu/Emacs/TestDBus")))) Oops. This shall be detected in dbus-register-property. > Cheers, > Hugh Best regards, Michael.