From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp11.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id uFuGH9G13WGGSwAAgWs5BA (envelope-from ) for ; Tue, 11 Jan 2022 17:52:33 +0100 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp11.migadu.com with LMTPS id sO7aHNG13WGHiAAA9RJhRA (envelope-from ) for ; Tue, 11 Jan 2022 17:52:33 +0100 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 1C97425145 for ; Tue, 11 Jan 2022 17:52:33 +0100 (CET) Received: from localhost ([::1]:44744 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n7KNs-0005nj-B1 for larch@yhetil.org; Tue, 11 Jan 2022 11:52:32 -0500 Received: from eggs.gnu.org ([209.51.188.92]:57766) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n7KHr-0001FY-Pr for emacs-orgmode@gnu.org; Tue, 11 Jan 2022 11:46:20 -0500 Received: from ciao.gmane.io ([116.202.254.214]:47814) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n7KHq-0003Nh-5B for emacs-orgmode@gnu.org; Tue, 11 Jan 2022 11:46:19 -0500 Received: from list by ciao.gmane.io with local (Exim 4.92) (envelope-from ) id 1n7KHo-00045t-ON for emacs-orgmode@gnu.org; Tue, 11 Jan 2022 17:46:16 +0100 X-Injected-Via-Gmane: http://gmane.org/ To: emacs-orgmode@gnu.org From: Max Nikulin Subject: Re: [PATCH] make test: Make failure results more verbose Date: Tue, 11 Jan 2022 23:46:08 +0700 Message-ID: References: <87ee5q5lic.fsf@localhost> <87lezrr3i7.fsf@localhost> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 In-Reply-To: <87lezrr3i7.fsf@localhost> Content-Language: en-US Received-SPF: pass client-ip=116.202.254.214; envelope-from=geo-emacs-orgmode@m.gmane-mx.org; helo=ciao.gmane.io X-Spam_score_int: 28 X-Spam_score: 2.8 X-Spam_bar: ++ X-Spam_report: (2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_ADSP_CUSTOM_MED=0.001, FORGED_GMAIL_RCVD=1, FORGED_MUA_MOZILLA=2.309, FREEMAIL_FORGED_FROMDOMAIN=0.248, FREEMAIL_FROM=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.249, NICE_REPLY_A=-0.001, NML_ADSP_CUSTOM_MED=0.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Migadu-Flow: FLOW_IN X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1641919953; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=vJMbqnLJONh3eBZ6DVraqikBUqH6QhucJsUCZnsW/zo=; b=jHAAOPvzF74/qkkzXpP4AcJUKRbaPQefoxL6VQJaALOHgj9pNnQrOwbzpjPyCBtlRVgtox CDfag78vQoVm+rIWOXh50IfxRxxbL4NMYS59ggbESqfkXeBIQlVGIGtQ11NnDytb89U1iU isgg+62cBvQLGYzV6w5X+5oyfCm0YMUs9CvJ49lhJj9BIQbuSExso7BQUrAro4N/oha/KF 7YozdUkWAKjq/m7+SvSBmh/RefvITSAb5OW7BNXyeSID3ekt0JUn743rNcorPz9t3TFD8U xiPAVc6B6hLgEb+ibtjWu/uiZt4o9twtBqqZIaNlNBXXqJlQtodjCp2Zmhy9dw== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1641919953; a=rsa-sha256; cv=none; b=mQlEQG0i81DYulPXnt3XoMEwww8T1a1fkzibNzbdiLqrMNbFpii/DObao7HeamBCVPmGDN WkMQGIcQHEf5T8K/7X16yBZjk0BSo1nWMF5orO9dkY8m59TNSTeEmSx4IJOt2ZHbtregtl bxCQiUE5qXJBLLDUOwNBNUBDdC0WkNwKEOkgM2L8+rj1BiKaZMyzIx4J+vb8wx6ahcccBZ ClHR0m41QcentOOc2G8e1qyzO4DSgpH0EFXXv6WBmt+ga1YsdMMeDXHYwSG+h9hNJMQ1YA 2WYtoaUUXh6B8M5qGfMpl02iUd1c13iGmCRopsB0RhuucwJ4baxK/K8mbA6Ufg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=gmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: -3.02 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=gmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: 1C97425145 X-Spam-Score: -3.02 X-Migadu-Scanner: scn0.migadu.com X-TUID: D8GYi+Rz8E5s On 07/01/2022 22:04, Ihor Radchenko wrote: > Max Nikulin writes: > >> Ihor, are there examples of new error reports in mail lists, blogs, etc? >> I am not motivated enough to try development version of emacs, but my >> impression that current error log looks like rectangles of garbage. > > Yeah, I should have attached examples to the original message. > Also, we can adjust the ERT output using > ert-batch-backtrace-right-margin, ert-batch-print-level, > ert-batch-print-level, and ert-batch-backtrace-line-length > > Here are the examples: Thank you. After sending that message I realized that I did not try to load just ert.el from git to emacs version that I have installed. >> In my opinion, code of test should be written having clear error reports >> in mind. > > I guess. Though I feel that ERT is better when used interactively. Your patch improves batch error reporting. Good error message should allow to figure out the origin of the problem without interactive debugging. > Do you have good ideas what could be changed? Side note. In f0c474e659b81da0d2ab75e7ec109355965f7a1c I have noticed "* H1\nP1\n* H2\n" I do not remember where I have seen tests with rather ugly failure reports. Example of what I consider as minor improvement (sorry for the wrapped text): (ert-deftest tst-inline-tags-orig () (should (equal '("foo" "baz") (progn (require 'org-inlinetask) (org-test-with-temp-text (concat (make-string org-inlinetask-min-level ?*) " Test :foo:bar:") (org-get-tags (org-element-at-point))))))) (defun org-test-no-properties (arg) (cond ((stringp arg) (org-no-properties arg)) ((listp arg) (mapcar #'org-test-no-properties arg)) (t arg))) (ert-deftest tst-inline-tags-new () (let ((inline-stars (make-string org-inlinetask-min-level ?*))) (require 'org-inlinetask) (should (equal '("foo" "baz") (org-test-with-temp-text (concat inline-stars " Test :foo:bar:") (org-test-no-properties (org-get-tags (org-element-at-point)))))))) F tst-inline-tags-new (ert-test-failed ((should (equal '("foo" "baz") (org-test-with-temp-text (concat inline-stars " Test :foo:bar:") (org-test-no-properties ...)))) :form (equal ("foo" "baz") ("foo" "bar")) :value nil :explanation (list-elt 1 (array-elt 2 (different-atoms (122 "#x7a" "?z") (114 "#x72" "?r")))))) F tst-inline-tags-orig (ert-test-failed ((should (equal '("foo" "baz") (progn (require ...) (org-test-with-temp-text ... ...)))) :form (equal ("foo" "baz") (#("foo" 0 3 (keymap ... mouse-face highlight font-lock-fontified t face ...)) #("bar" 0 3 (keymap ... mouse-face highlight font-lock-fontified t face ...)))) :value nil :explanation (list-elt 1 (array-elt 2 (different-atoms (122 "#x7a" "?z") (114 "#x72" "?r")))))) >>> +BTEST_ERT_VERBOSE = yes >> >> I am unsure if this line or local.mk has priority. I am unsure the the >> following is better as well. >> BTEST_ERT_VERBOSE ?= yes > > I am not very familiar with Makefile conventions. Just followed the > existing settings in the same file. All other BTEST_ERT_* settings just > use "=". OK, maybe I will try it later. I just do not remember in which order assignments are evaluated. Likely everything works as expected and features like delayed assignment (simple "=" vs. ":=" ) do not cause any problem. >> Is there an easy way to limit number of failures before termination of >> tests in the case of verbose reporting? It should prevent test log from >> blowing too much. Usually there is no point in all details if all or >> even 1/4 of tests fails. > > The best approach I know is using BTEST_RE BTEST_RE is useful when you know in advance which tests you are interested in. I had in mind continuous integration and notifications. Single full error (or several ones) included into a message is convenient and may provide enough information for quick fix. If there are 500 errors there is no point to send all of them as notification. Either test is aborted after a couple of dozens of failures or total number of failures and full text of several errors are enough to avoid excessively long notification. Anyway nobody is going to read all hundreds of failures. >>> + TMPDIR=$(testdir) EMACS_TEST_VERBOSE=yes $(BTEST) >> >> A purist would say that it is not a directory, it is something like >> ...FLAGS or ...ARGS. I know, it was abused before your patch. > > I do not follow you here. > VARIABLE1=1 VARIABLE2=2 /path/to/script > is the usual way to set variables in script environment in bash. My bad. I did not noticed that it is a part of script of a rule. I believed that the whole line is assignment to TMPDIR variable. I am unsure if the following approach to avoid excessive ifeq's without modification of the rule is better: ifeq ($(BTEST_ERT_VERBOSE),yes) EMACS_TEST_VERBOSE=yes export EMACS_TEST_VERBOSE endif >> Shouldn't it be mentioned in testing/README? > > Only BTEST_RE is documented there. BTEST_PRE, BTEST_POST, > BTEST_OB_LANGUAGES, and BTEST_EXTRA are not documented. > I believe that the odds for the user to change BTEST_ERT_VERBOSE are > rather low and it is not worth mentioning it explicitly in README. > > Or, alternatively, we can document all the settings in README. > WDYT? I agree that default verbose log is preferred in most cases. The only exception when it becomes rather useless and just consumes disk space is something basic is broken (see my comment concerning limit on failures above). Let's assume that you convinced me and only rare developers will customize the value.