From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54979) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fcX3H-00036y-U1 for guix-patches@gnu.org; Mon, 09 Jul 2018 10:22:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fcX3C-00084W-QR for guix-patches@gnu.org; Mon, 09 Jul 2018 10:22:07 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:44651) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fcX3C-00084P-MQ for guix-patches@gnu.org; Mon, 09 Jul 2018 10:22:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fcX3C-0005Un-DZ for guix-patches@gnu.org; Mon, 09 Jul 2018 10:22:02 -0400 Subject: [bug#31911] services: Add prometheus-node-exporter-service-type. Resent-Message-ID: References: <20180620125946.7264-1-boskovits@gmail.com> <87sh4s93ai.fsf@lassieur.org> From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur In-reply-to: Date: Mon, 09 Jul 2018 16:21:44 +0200 Message-ID: <87pnzw8p6v.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: =?UTF-8?Q?G=C3=A1bor?= Boskovits Cc: 31911-done@debbugs.gnu.org G=C3=A1bor Boskovits writes: > Cl=C3=A9ment Lassieur ezt =C3=ADrta (id=C5=91pont:= 2018. j=C3=BAl. 9., > H, 11:17): > >> Hi G=C3=A1bor, >> >> G=C3=A1bor Boskovits writes: >> >> > Pushed to master as a33652ee336ae9a5d2ab5fd54bf2397caec42a0e. >> >> > + (test-assert "prometheus-node-exporter running" >> > + (marionette-eval >> > + '(begin >> > + (use-modules (gnu services herd)) >> > + (match (start-service 'prometheus-node-exporter) >> > + (#f #f) >> > + (('service response-parts ...) >> > + (match (assq-ref response-parts 'running) >> > + ((pid) (number? pid)))))) >> > + marionette)) >> >> The PID check is useless because START-SERVICE will return #f if the >> service fails to start. Instead, I'd use: >> >> (test-assert "prometheus-node-exporter running" >> (marionette-eval >> '(begin >> (use-modules (gnu services herd)) >> (start-service 'prometheus-node-exporter)) >> marionette)) >> >> This would also make the test more robust to service changes (e.g. it >> would still work if MAKE-FORKEXEC-CONSTRUCTOR is removed). >> >> > Thanks, I will adjust accordingly. Incidentally the code is almost the sa= me > as in hpcguix-web test. Should we also adjust that? You're welcome :-) It's not urgent, but in the long term, it would be good that all similar code is ajusted accordingly. It's in lots of other places as well if I remember well.