From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Hugh Daschbach Newsgroups: gmane.emacs.bugs Subject: bug#43252: 27.1; DBus properties lack type hints or overrides Date: Tue, 22 Sep 2020 20:30:47 -0700 Message-ID: <87wo0l8908.fsf@ccss.com> References: <87v9gqquct.fsf@ccss.com> <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> <87h7rva0ku.fsf@gmx.de> <875z8aa1bm.fsf@ccss.com> <87ft7c8p4k.fsf@gmx.de> <87imc7jqms.fsf@gmx.de> <87zh5i8o9h.fsf@ccss.com> <87eemt67eg.fsf@gmx.de> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="38140"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: mu4e 1.5.5; emacs 27.1 Cc: 43252@debbugs.gnu.org To: Michael Albinus Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Sep 23 05:32:11 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 1kKvVr-0009nr-9e for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 23 Sep 2020 05:32:11 +0200 Original-Received: from localhost ([::1]:35894 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kKvVp-0001aP-Rs for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 22 Sep 2020 23:32:09 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:36104) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kKvVi-0001a4-8K for bug-gnu-emacs@gnu.org; Tue, 22 Sep 2020 23:32:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:50829) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kKvVh-0001jg-Tp for bug-gnu-emacs@gnu.org; Tue, 22 Sep 2020 23:32:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kKvVh-0005Pm-RH for bug-gnu-emacs@gnu.org; Tue, 22 Sep 2020 23:32:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Hugh Daschbach Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 23 Sep 2020 03:32:01 +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.160083186820625 (code B ref 43252); Wed, 23 Sep 2020 03:32:01 +0000 Original-Received: (at 43252) by debbugs.gnu.org; 23 Sep 2020 03:31:08 +0000 Original-Received: from localhost ([127.0.0.1]:34116 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kKvUo-0005MY-NS for submit@debbugs.gnu.org; Tue, 22 Sep 2020 23:31:08 -0400 Original-Received: from mail1.ccss.com ([159.203.255.73]:60796) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kKvUj-0005Lo-Gk for 43252@debbugs.gnu.org; Tue, 22 Sep 2020 23:31:05 -0400 Original-Received: by mail1.ccss.com (Postfix, from userid 114) id 8C571BF889; Tue, 22 Sep 2020 20:30:55 -0700 (PDT) Original-Received: from ccss.com (unknown [192.168.76.11]) by mail1.ccss.com (Postfix) with ESMTP id A4039BF475; Tue, 22 Sep 2020 20:30:54 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by ccss.com (Postfix) with ESMTP id 7C90B176063A; Tue, 22 Sep 2020 20:30:54 -0700 (PDT) Original-Received: from ccss.com ([127.0.0.1]) by localhost (ccss.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id kC3FHLHNiyPx; Tue, 22 Sep 2020 20:30:48 -0700 (PDT) Original-Received: from klaatu (klaatu.lan [192.168.42.3]) (Authenticated sender: hugh) by ccss.com (Postfix) with ESMTPSA id ED5E7176042D; Tue, 22 Sep 2020 20:30:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ccss.com; s=mail; t=1600831848; bh=gttxXWDtvQYatGndhFuAHUVlfo942nvy+nH8PbnvYVU=; h=References:From:To:Cc:Subject:In-reply-to:Date:From; b=hnraslPrQGVQFxvOduTuhuKLboK1wsgN3n/giKJdtopL9q8+mJ2IF6eG42p7cD6Id oNPN8lZE7jtfX04sr2DwwjfY47utDxOmacAYrMlVncEoKpQwehlaPCb/ayPB8j03FH G1bkMRPhbncvQgl7tlyT4lKRp5QFAt8YfwZ5KlVrjZqFEijjqxpDFlRG+kGHaLgDJZ m/YA5A/W6wwNlyqfREudR2Vrqu3ztbNlF++HyAtzB7zmXmU0myJO8ufWU7QzAGevnW 2JYLl5qjE+GIVnuDYWnD6G6A0S9tbDmtkslNquaRnCbguViORsm3w7gAQg3SzQigva HEKlUlcqDuqFA== In-reply-to: <87eemt67eg.fsf@gmx.de> 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:188756 Archived-At: Michael Albinus writes: > Hugh Daschbach writes: > > Hi Hugh, > > Thanks for this. AFAICS, there's nothing left open for=20 > bug#43252; I'd > like to close it. Is this OK for you? Yes. I was about to suggest this. The issue is resoled. >> And, of course, let me know what you think should be reworked. > > There are only some nits left to comment. Good. Nits promote quality and consistency. >> Add DBus tests to validate property handling. Includes cycling > > We call this beast D-Bus, with a hyphen. Here and everywhere in=20 > the > docstrings and comments. Good. Fixed. > > We add also a ChangeLog style entry to the commit message, see=20 > the git > log of dbus-tests.el. I've taken a shot at this. I'm not sure I got the ChangeLog style correct. Let me know if I'm still off the beaten path. > `dbus-register' (if SELECTOR is `register') or Fixed. >> +(ert-deftest dbus-test06-test-property-types () > > The "-test" part of the name seems to be superfluous; I'd call=20 > it > dbus-test06-property-types. (You see, just nitpicks :-) You say that as if were a bad thing :-). Fixed. >> + (dbus--test-property >> + "Dictionary" >> + '((:array >> + :dict-entry (:string "four" (:variant :string=20 >> "value of four")) >> + :dict-entry ("five" (:variant :object-path=20 >> "/nodex")) >> + :dict-entry ("six" (:variant (:array :byte 4=20 >> :byte 5 :byte 6)))) > > This is one possibility to declare a :dict-entry. The other=20 > possibility, > with the same result, is > > '((:array > (:dict-entry :string "four" (:variant :string "value of=20 > four")) > (:dict-entry "five" (:variant :object-path "/nodex")) > (:dict-entry "six" (:variant (:array :byte 4 :byte 5 :byte=20 > 6)))) > ... > > Do you mind to test both? I seem to consistently stumble over compound type syntax. Thanks=20 for point this out. Both forms are now tested. >> + (should-error ; Cannot set property=20 >> with :read access >> + (equal ...)) > > The error happens in dbus-set-property. No need to test further=20 > for > equal. So you could do > > (should-error ; Cannot set property with :read=20 > access > (dbus-set-property > :session dbus--test-service dbus--test-path > dbus--test-interface "StringArray" > '(:array "seven" "eight" :string "nine"))) > > Furthermore, you could test which error is raised (should-error=20 > allows > this). Something like > > (should-error ; Cannot set property with :read=20 > access > (dbus-set-property > :session dbus--test-service dbus--test-path > dbus--test-interface "StringArray" > '(:array "seven" "eight" :string "nine")) > :type 'dbus-error) > > Similar approach for your other should-error forms. Got it. I think I've fixed each should-error test. >> + ;; Test integer overflow > > I don't see any integer *overflow* in the following tests. Yes, really botched that. I had a byte truncation test that I=20 only now realize isn't treated as an integer. And that wasn't near the=20 comment. Sigh. I've added conforming and overflow tests. The overflow test, appropriately, error on register. > OTOH, I don't mind to give this test the :expensive-test=20 > tag. Then it > doesn't matter how long it runs. Done. > Given the time it consumes, there might be a need to cache=20 > introspection > data. Either the result of dbus-introspect or=20 > dbus--parse-xml-buffer, I > guess rather the latter. Do you want to investigate it in=20 > dbus.el? Sure. I'll have a look. If I find something useful, I'll open=20 another bug. >> And, of course, let me know what you think should be reworked. > > Here we are. I don't repeat general comments I have given the=20 > other review. > >> +(defun dbus--test-introspect () >> + "Return test introspection string." >> + " > > ... > > Well, this is one approach. Alternatively, we could regard the > introspection file as test data, which is located in a file=20 > called > .../test/lisp/net/dbus-tests/org.gnu.Emacs.TestDBus.xml. This=20 > function > (the handler for the Introspect method) would read the file, and=20 > return > its contents. Makes sense. Done. >> +(defsubst dbus--test-examine-interface (iface-name >> + expected-properties >> + expected-methods >> + expected-signals >> + expected-annotations) > > This is rather C-style of argument indentation. In ELisp, we do > something like Guilty as charged. I'm still working on developing idiomatic=20 elisp. > (defsubst dbus--test-examine-interface > (iface-name expected-properties expected-methods > expected-signals expected-annotations) > ...) > >> + (let ((interface (dbus-introspect-get-interface >> + :session >> + dbus--test-service >> + dbus--test-path >> + iface-name))) > > A similar comment applies. Is: (let* ((property (dbus-introspect-get-property :session dbus--test-service dbus--test-path interface property-name))) ...) preferred over: (let* ((property (dbus-introspect-get-property :session=20 dbus--test-service dbus--test-path interface property-name))) ...) If not, I'll take another bite at the apple. >> + (should-not (equal expected nil)) > > This is (should expected) Yes, I should read what I write. Fixed. Thanks. >> + (unwind-protect >> + ;; dbus-introspect-get-node-names >> + (should >> + (equal >> + (dbus-introspect-get-node-names :session=20 >> dbus--test-service dbus--test-path) >> + '("node0" "node1"))) > A (progn ... is missing after unwind-protect. I really should be more careful when I rip out instrumentation.=20 Thanks for catching this. >> + '("org.freedesktop.DBus.Deprecated"))) > > Hmm. Maybe we shall give "org.freedesktop.DBus.Deprecated" a=20 > defconst in > dbus.el? Done. =E2=80=99dbus-annotation-deprecated=E2=80=99. Let me know if you th= ink=20 this should be =E2=80=98dbus--annotation-deprecated=E2=80=99 instead. >> Thanks, >> Hugh > > Best regards, Michael. In other news, this test: (should-error (dbus-check-arguments :session dbus--test-service :unix-fd 10) :type 'dbus-error) works for me in batch mode, but not interactive mode. On GNU/Linux, (dired (format "/proc/%s/fd" (emacs-pid))) indicates=20 I have 14 open file descriptors on a test instance (emacs -q -Q).=20 On my development instance, I've got 27 open file descriptors. Cheers, Hugh