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: Tue, 11 Jul 2017 21:47:44 -0600 Message-ID: <87vamyl3j3.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> <87o9sywtbz.fsf@lylat> <87fue3f9p8.fsf@users.sourceforge.net> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1499831304 8810 195.159.176.226 (12 Jul 2017 03:48:24 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 12 Jul 2017 03:48:24 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) Cc: Gemini Lasswell , Tino Calancha , 24402@debbugs.gnu.org To: npostavs@users.sourceforge.net Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Jul 12 05:48:15 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 1dV8dG-0001KF-7E for geb-bug-gnu-emacs@m.gmane.org; Wed, 12 Jul 2017 05:48:10 +0200 Original-Received: from localhost ([::1]:50147 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dV8dI-00061R-8I for geb-bug-gnu-emacs@m.gmane.org; Tue, 11 Jul 2017 23:48:12 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:60114) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dV8dC-000619-3f for bug-gnu-emacs@gnu.org; Tue, 11 Jul 2017 23:48:07 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dV8d8-0003bE-1J for bug-gnu-emacs@gnu.org; Tue, 11 Jul 2017 23:48:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:60123) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dV8d7-0003b6-Sx for bug-gnu-emacs@gnu.org; Tue, 11 Jul 2017 23:48:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1dV8d7-0007vL-JR for bug-gnu-emacs@gnu.org; Tue, 11 Jul 2017 23:48: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, 12 Jul 2017 03:48: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.149983127530445 (code B ref 24402); Wed, 12 Jul 2017 03:48:01 +0000 Original-Received: (at 24402) by debbugs.gnu.org; 12 Jul 2017 03:47:55 +0000 Original-Received: from localhost ([127.0.0.1]:34567 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dV8d1-0007uy-KK for submit@debbugs.gnu.org; Tue, 11 Jul 2017 23:47:55 -0400 Original-Received: from mail-io0-f195.google.com ([209.85.223.195]:35947) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dV8d0-0007un-GL for 24402@debbugs.gnu.org; Tue, 11 Jul 2017 23:47:55 -0400 Original-Received: by mail-io0-f195.google.com with SMTP id h134so1036156iof.3 for <24402@debbugs.gnu.org>; Tue, 11 Jul 2017 20:47:54 -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=YttQ6BmdnrvQqRAs8Y0ohl2RuG8jo0P9/GhrCSAvYnw=; b=XyCgS/V31uObG81HmNKeuNLyZlPbhkWhdUTZXvrZEDAgvghWXlk3xmpJvra8fj0kgi 6FFzb51tFBJSW9A11r4EsMVTidQzzVrK4qX9w/ck4AlacbNWWnkpwQoXIvSo6x1/vrUQ Tq/sa2uAjdmAy+n7c1AiCwyRV9QhPwCj2xGm4wuUFFuzdlvYuqQaUnodGlR0CmYCSKJA bunYH6jmZ+3q/Md7PvJvHAHvPoCLAjqzSti3tCWJaq+XFoVdK8FIroZgnQ87lQ+unREo NExPa3JY52U3EBz7XzybjfDZ5g8dUUIl7DmP7xTI8AlpK7VdCNWLaLpPr768uh910hRS GRPg== 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=YttQ6BmdnrvQqRAs8Y0ohl2RuG8jo0P9/GhrCSAvYnw=; b=ml0Y4/+vYNGsVxKlLrT+bet7v/4CbcvAzpKoPRmL83fbPfjvc+5P2ZsJZw86QJZtoN vAJgyoMGqaduPr3hK23kBUgwlKN/K/zwF76SotH5/Ldx5e1aQQLNGw4JSm5q+j0Z/AO7 hePiur8V4oXVmsSrwFEWfiRjPlfWud2ODkzP0jc6N+/0dmh3N+deZWjEo/lw6JQtwdjm FBbZKqH3hMstCA0h+AQ/RfHcK6Zu4jPgA4JJpJyIkRQJ0pgG7isyEBBwETC5dvT6BxHx ofI45ILQh2itQGuoaqcOwCl5iDD9KAIiQVruQvxG/WUyaE3gTGq2lqWwJYiGK9CUvYRM 6X/A== X-Gm-Message-State: AIVw113T2yEB7WxFuag1tZ2CHEWfZmx5aBSZ/Ol+JD7aPSJ14JWrtHoD UsfYfp5KSom/EA== X-Received: by 10.107.178.214 with SMTP id b205mr3593963iof.68.1499831267797; Tue, 11 Jul 2017 20:47:47 -0700 (PDT) Original-Received: from lylat (S010664777d9cebe3.ss.shawcable.net. [70.64.85.59]) by smtp.gmail.com with ESMTPSA id k16sm666594itb.1.2017.07.11.20.47.45 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 11 Jul 2017 20:47:46 -0700 (PDT) In-Reply-To: <87fue3f9p8.fsf@users.sourceforge.net> (npostavs's message of "Tue, 11 Jul 2017 08:18:43 -0400") 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:134449 Archived-At: --=-=-= Content-Type: text/plain npostavs@users.sourceforge.net writes: > Yes, ert binds `debugger' in order to get full backtrace information > when there is an error. This means it won't see errors caught by > condition-case. That's good when it ignores errors caught by test code > using condition-case, but does give rise to problems. There is some > relevant discussion in Bugs #11218 and #24617. > > Espcially the suggestion in #24617 of using `signal-hook-function' to > record error info instead of using `debugger', I think doing this could > simplify things a lot. It is definitely going to require messing around > with ert's internals though... Thanks for the info. I may have discovered a workaround, but I'm not sure if there's any negative side-effects. All the tests pass, though. What do you think of it? It's obviously not ideal, but I think it at least fixes the issues at hand. --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=ert-3.diff Content-Description: v3 diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el index eb2b2e3e11..beb32c48ce 100644 --- a/lisp/emacs-lisp/ert.el +++ b/lisp/emacs-lisp/ert.el @@ -266,6 +266,14 @@ ert--signal-should-execution (when ert--should-execution-observer (funcall ert--should-execution-observer form-description))) +;; See Bug#24402 for why this exists +(defun ert--signal-hook (error-symbol data) + "Stupid hack to stop `condition-case' from catching ert signals. +It should only be stopped when ran from inside ert--run-test-internal." + (when (and (not (symbolp debugger)) ; only run on anonymous procedures + (memq error-symbol '(ert-test-failed ert-test-skipped))) + (funcall debugger 'error data))) + (defun ert--special-operator-p (thing) "Return non-nil if THING is a symbol naming a special operator." (and (symbolp thing) @@ -276,13 +284,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 +313,13 @@ 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 + (let ((signal-hook-function #'ert--signal-hook)) + (list ,@arg-forms)) + (error (progn (setq ,fn #'signal) + (list (car err) + (cdr err))))))) (let ((,value ',default-value)) ,(funcall inner-expander `(setq ,value (apply ,fn ,args)) diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el index 317838b250..b6a7eb68da 100644 --- a/test/lisp/emacs-lisp/ert-tests.el +++ b/test/lisp/emacs-lisp/ert-tests.el @@ -294,6 +294,15 @@ ert-self-test-and-exit "the error signaled was a subtype of the expected type"))))) )) +(ert-deftest ert-test-should-error-argument () + "Errors due to evaluating arguments should not break tests." + (should-error (identity (/ 1 0)))) + +(ert-deftest ert-test-should-error-macroexpansion () + "Errors due to expanding macros should not break tests." + (cl-macrolet ((test () (error "Foo"))) + (should-error (test)))) + (ert-deftest ert-test-skip-unless () ;; Don't skip. (let ((test (make-ert-test :body (lambda () (skip-unless t))))) --=-=-=--