From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Alex Newsgroups: gmane.emacs.bugs Subject: bug#24402: should-error doesn't catch all errors Date: Wed, 05 Jul 2017 13:56:48 -0600 Message-ID: <87o9sywtbz.fsf@lylat> References: <3654D8E9-D3CB-402B-922F-B132C1871E9F@runbox.com> <596E65D2-E780-43A1-A75B-603B61B6F9F4@runbox.com> <87zickhoco.fsf_-_@lylat> <877eznda7v.fsf@lylat> <874lur0zki.fsf@calancha-pc> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1499284691 10289 195.159.176.226 (5 Jul 2017 19:58:11 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 5 Jul 2017 19:58:11 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) Cc: Gemini Lasswell , 24402@debbugs.gnu.org To: Tino Calancha Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Jul 05 21:58:07 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dSqR4-0002RY-RA for geb-bug-gnu-emacs@m.gmane.org; Wed, 05 Jul 2017 21:58:07 +0200 Original-Received: from localhost ([::1]:47857 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dSqRA-00041X-B9 for geb-bug-gnu-emacs@m.gmane.org; Wed, 05 Jul 2017 15:58:12 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:51419) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dSqR4-00041P-8a for bug-gnu-emacs@gnu.org; Wed, 05 Jul 2017 15:58:07 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dSqR0-0002st-At for bug-gnu-emacs@gnu.org; Wed, 05 Jul 2017 15:58:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:51073) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dSqR0-0002sn-6L for bug-gnu-emacs@gnu.org; Wed, 05 Jul 2017 15:58:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1dSqQz-0004F8-M0 for bug-gnu-emacs@gnu.org; Wed, 05 Jul 2017 15:58:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Alex Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 05 Jul 2017 19:58:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 24402 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: confirmed patch Original-Received: via spool by 24402-submit@debbugs.gnu.org id=B24402.149928462316243 (code B ref 24402); Wed, 05 Jul 2017 19:58:01 +0000 Original-Received: (at 24402) by debbugs.gnu.org; 5 Jul 2017 19:57:03 +0000 Original-Received: from localhost ([127.0.0.1]:53750 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dSqQ2-0004Dv-TV for submit@debbugs.gnu.org; Wed, 05 Jul 2017 15:57:03 -0400 Original-Received: from mail-it0-f66.google.com ([209.85.214.66]:32846) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dSqQ0-0004DR-Qr for 24402@debbugs.gnu.org; Wed, 05 Jul 2017 15:57:01 -0400 Original-Received: by mail-it0-f66.google.com with SMTP id 188so15442748itx.0 for <24402@debbugs.gnu.org>; Wed, 05 Jul 2017 12:57:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=PrmOamwlTRfx+NnW74jzgiv6sH/Tf6cmh96SQF15OQg=; b=H1a423CZyDIOByqLqqYu/hvl0HyfCjV3RgDFb4HeGZTBy6bKRTu3FJbOz3Tflr9dPI O+r5YsdqHgYahAeOEIP4gOwmFvPM3pQJ41q8Q6zWk0XN/TlYX+cpkikyB/62ULLB+RQU J6/6nBa4k4C1iCkHPEfZw3uYZpCH6622qF39P9KupyHWT6wzU2tMjUpQMfNCt8Xa12g3 oT7q6/2xjJSWP0WqKhNlUkP/mVhiKNEyY+/1Nz+V1scPZ/c1vetH3klRipIHtS2b7OEC po9grR22Sa9+GaEKl4rfabXCEkwAAWsvtGd/iNV5NPTaaG8PxivWrUCAy/RmfM5izydD ZA7g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=PrmOamwlTRfx+NnW74jzgiv6sH/Tf6cmh96SQF15OQg=; b=M7edIMV/L2vuIq+9D3ktq4aK4l1x4zIUp3UfILJyeqr6KADADNfTfdrhCaIsfBF2DR sG33RJmZTBATn3RKiyV3d2Cy2jvf143RsapLmCQ9fkgt/HOcD3Qg2MppEKY2+F9f0ufO 2MshJk0Dq10GsamHUfyvNMtE09diAE95xRTjzYVYtsfoszEw5Ak/4BiUXphr/J0KptHt EwWhVL1shESsc0VD3m1Pz4IWb7pITSlyxwa4jLiWtoYbCongF3V8K0dHMQg2b7cXLObb gOZ7c712I568ZRZwUvXbbAaXWmU0Vk2JoWH5vve14t5hQvJP4zS1jpuujJEX1DkDbdPE X/aw== X-Gm-Message-State: AKS2vOzTVRFv/dIdWAzDCGnUIfjUpqodANZKYTLtvJMLnWYokKKZ7XiG l2Pj/vFhCgEzH84y X-Received: by 10.36.93.142 with SMTP id w136mr44541319ita.13.1499284614944; Wed, 05 Jul 2017 12:56:54 -0700 (PDT) Original-Received: from lylat (S010664777d9cebe3.ss.shawcable.net. [70.64.85.59]) by smtp.gmail.com with ESMTPSA id c4sm6505666ioc.18.2017.07.05.12.56.53 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 05 Jul 2017 12:56:53 -0700 (PDT) In-Reply-To: <874lur0zki.fsf@calancha-pc> (Tino Calancha's message of "Wed, 05 Jul 2017 22:43:09 +0900") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 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.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:134223 Archived-At: Tino Calancha writes: > Hi! > I just arrived from teletransportation from Bug#27559. Very fast! (and > cheap!). > > Thank you for looking on this. I think you go in the right direction to > fix this problem. > > * I have updated your patch and all the test suite pass (even your > proposed tests in Bug#27559 without requiring "(eval '....)"). > > * Bear in mind that I am far to be an expert on `ert.el', and i am > already in my second beer, so please double check that > the patch have sense. > > > commit a07f99f062f3da3418060ef30e1a00030fa0b947 > Author: Tino Calancha > Date: Wed Jul 5 22:11:46 2017 +0900 > > Tweak Alex's 2nd patch > > diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el > index eb2b2e3e11..2d131cf99e 100644 > --- a/lisp/emacs-lisp/ert.el > +++ b/lisp/emacs-lisp/ert.el > @@ -276,13 +276,15 @@ ert--special-operator-p > (defun ert--expand-should-1 (whole form inner-expander) > "Helper function for the `should' macro and its variants." > (let ((form > - (macroexpand form (append (bound-and-true-p > - byte-compile-macro-environment) > - (cond > - ((boundp 'macroexpand-all-environment) > - macroexpand-all-environment) > - ((boundp 'cl-macro-environment) > - cl-macro-environment)))))) > + (condition-case err > + (macroexpand-all form (append (bound-and-true-p > + byte-compile-macro-environment) > + (cond > + ((boundp 'macroexpand-all-environment) > + macroexpand-all-environment) > + ((boundp 'cl-macro-environment) > + cl-macro-environment)))) > + (error `(signal ',(car err) ',(cdr err)))))) > (cond > ((or (atom form) (ert--special-operator-p (car form))) > (let ((value (cl-gensym "value-"))) > @@ -303,8 +305,14 @@ ert--expand-should-1 > (args (cl-gensym "args-")) > (value (cl-gensym "value-")) > (default-value (cl-gensym "ert-form-evaluation-aborted-"))) > - `(let ((,fn (function ,fn-name)) > - (,args (list ,@arg-forms))) > + `(let* ((,fn (function ,fn-name)) > + (,args (condition-case err > + (list ,@arg-forms) > + (error (if (or (eq (car err) 'ert-test-failed) > + (eq (car err) 'ert-test-skipped)) > + (list ,@arg-forms) > + (setq ,fn #'signal) > + (list (car err) (cdr err))))))) > (let ((,value ',default-value)) > ,(funcall inner-expander > `(setq ,value (apply ,fn ,args)) Thanks, that helps with my understanding of the issue. Sadly, I don't think your tweak will work in all cases, namely whenever (list ,@arg-forms) has side-effects. Luckily this doesn't happen in any current tests, but I think those types of tests should be supported. I believe the following is why my previous diff doesn't work. Consider: (let ((test (make-ert-test :body (lambda () (ert-fail "failed"))))) (ert-run-test test)) The above returns a struct/record and does not error. (let ((test (make-ert-test :body (lambda () (ert-fail "failed"))))) (condition-case err (ert-run-test test) (error "woops"))) Even though ert-run-test by itself does not error, the error handler is ran. I believe this is because `ert--run-test-internal' binds `debugger' around the evaluation of the test to somehow suppress this error. Using condition-case-unless-debug gets around this, but now anytime debug-on-error is non-nil the condition-case won't catch the error. I'm not sure how to solve this.