GNU bug report logs - #77082
[PATCH] Add some more Eshell history tests

Message received at submit <at> debbugs.gnu.org:

Received: (at submit) by debbugs.gnu.org; 17 Mar 2025 19:52:38 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Mon Mar 17 15:52:37 2025
Received: from localhost ([127.0.0.1]:32845 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1tuGVp-0004eq-1b
	for submit <at> debbugs.gnu.org; Mon, 17 Mar 2025 15:52:37 -0400
Received: from lists.gnu.org ([2001:470:142::17]:53298)
 by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.84_2) (envelope-from <Morgan.J.Smith@HIDDEN>)
 id 1tuGVm-0004dm-FI
 for submit <at> debbugs.gnu.org; Mon, 17 Mar 2025 15:52:35 -0400
Received: from eggs.gnu.org ([2001:470:142:3::10])
 by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <Morgan.J.Smith@HIDDEN>)
 id 1tuGVb-0005RV-VF
 for bug-gnu-emacs@HIDDEN; Mon, 17 Mar 2025 15:52:24 -0400
Received: from mail-sn1nam02olkn2061.outbound.protection.outlook.com
 ([40.92.44.61] helo=NAM02-SN1-obe.outbound.protection.outlook.com)
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <Morgan.J.Smith@HIDDEN>)
 id 1tuGVZ-00058m-HS
 for bug-gnu-emacs@HIDDEN; Mon, 17 Mar 2025 15:52:23 -0400
ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none;
 b=OOIckWa2ptC2Subkg1YCSAS9S528NbOdVvlWwCN2XnxLmN4MDDKYhApg4fHZGi2DV/dtPunMORuUYUd77lr3RXsx29UjN7N3yl9gMlGZJaO4VliDg8oYBDEHiMS1sqm47WEJ4kZZ9wEQe1vYcNuStVvFQbSKBSNPOerE1BJTtsqR2fhxh4wckGcMiqb5ZXNpklNgd7sVeNGGrbyIzJQo3FM5uraXsRbhGvzL7iOKsLJTiyrDCeSkzFOxF38Fvx1BxY5MRF2m2jr62CF5ANKMHR8MhiVZaT61wqEHZmA7VFITefr4xIUvYhaM8PdvHZuLXON1GjIal9bwvfg3tqC73g==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; 
 s=arcselector10001;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;
 bh=BJkQsLC803rH25VQ1Zb4veiB+/QZ8VqnhMow/gqlHuA=;
 b=LUsUuYCVIw1+89AKKcR51pgVmR/buExBlEZe5gpPTpRBuuhFRFsD4G+lfUromxE60+9j4pIRMP8lqT/yuV6bg/VIEPNdumbS6w/x2fc6iZVZiNJuGzc/cNnct2S/oCd+n1FR9rF95/VtxDf1LAs7V1iCe3RnlpcQzFSJwZjLa6NhDrBxgjy5Eat0T4CHtIviaFi2xnr8utvWQ2apoWJEYKTfo2vcHMkI4wYDcb6uEu5Aev5ou3xJmdxreXKyLmcYVMRJYnR7k512lef/wxaONepICO2edWCxL4a6l8GNLu4tqNjXs933UCmMPY3h+78IB7LBf03/UbNMkkGtbm5aVA==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none;
 dkim=none; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com;
 s=selector1;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
 bh=BJkQsLC803rH25VQ1Zb4veiB+/QZ8VqnhMow/gqlHuA=;
 b=DTNstt9bEHyZL34nQWg8GSez3GAb6AnF5DBdSYgNkGm4Xt46RuemAucSMPeF3faBm57jOb3VMadWeWucRrNjXZH67nLLgH4Axm3tggMTNTGC435CFYhQWMGBzb0VLNLus4y9DXRnZHxDy0ie977w2BC9AWsFCDP1h76rJIVAxt5qmAdUFAcANXep4XE0SH9izayIAGgUQFn/6Y3Ba1FRyGTUxhvEvMqSaIyDHhzbLJTAZgKl3R0S9nabnLM+mkv86STKbF0AdvZHJvijBYHqVw7P+2jpB+V4kSWF8qrzEcqB+dSzcvjBYwURPVD8OLlZZcfSTFlqqqTdP+NvR+NNaw==
Received: from CH3PR84MB3424.NAMPRD84.PROD.OUTLOOK.COM (2603:10b6:610:1c4::17)
 by PH7PR84MB3282.NAMPRD84.PROD.OUTLOOK.COM (2603:10b6:510:12d::8)
 with Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8534.33; Mon, 17 Mar
 2025 19:52:18 +0000
Received: from CH3PR84MB3424.NAMPRD84.PROD.OUTLOOK.COM
 ([fe80::5c77:7a58:48ed:9aef]) by CH3PR84MB3424.NAMPRD84.PROD.OUTLOOK.COM
 ([fe80::5c77:7a58:48ed:9aef%4]) with mapi id 15.20.8534.031; Mon, 17 Mar 2025
 19:52:18 +0000
From: Morgan Smith <Morgan.J.Smith@HIDDEN>
To: bug-gnu-emacs@HIDDEN
Subject: [PATCH] Add some more Eshell history tests
X-Debbugs-Cc: 
Date: Mon, 17 Mar 2025 15:52:15 -0400
Message-ID: <CH3PR84MB3424FB78037373E0E513D70CC5DF2@HIDDEN>
Content-Type: multipart/mixed; boundary="=-=-="
X-ClientProxiedBy: YT4PR01CA0183.CANPRD01.PROD.OUTLOOK.COM
 (2603:10b6:b01:110::19) To CH3PR84MB3424.NAMPRD84.PROD.OUTLOOK.COM
 (2603:10b6:610:1c4::17)
X-Microsoft-Original-Message-ID: <871puvsads.fsf@HIDDEN>
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
X-MS-PublicTrafficType: Email
X-MS-TrafficTypeDiagnostic: CH3PR84MB3424:EE_|PH7PR84MB3282:EE_
X-MS-Office365-Filtering-Correlation-Id: e5faddde-01e7-463f-25a4-08dd658d3923
X-Microsoft-Antispam: BCL:0;
 ARA:14566002|461199028|8060799006|19110799003|5072599009|6092099012|7092599003|15080799006|13095399003|440099028|3412199025|12091999003;
X-Microsoft-Antispam-Message-Info: =?us-ascii?Q?+wHneqmG6dOJWkIrUlifpRbQO29VWnUt8VdrQjuQ56Esx183IJCcsjgJbnQX?=
 =?us-ascii?Q?7+qzEm58+m+/APc9RloXRO7pcMuoJEUk3FgWpAZtvtzvByzS7UDmYFUrEHXN?=
 =?us-ascii?Q?DjjneAq9SHc1lScWiyMYuh+lr/P1VOcnmxa68vAZ9tN4Z086Q4mw3QmToR3d?=
 =?us-ascii?Q?70Ek+EuUptGVz52vZDFjggyvFNkFyn+ts/yZk1h+Jrsw1MK48/3tCStI56pA?=
 =?us-ascii?Q?cZMvMfrHhEkaTNm4Tx4XYEs5WjBA9MDgEmIjIhhW7LoHe4CjqCIbzMqu5mxv?=
 =?us-ascii?Q?AWrTwPcA+Jj3K3OI/ok96ckpEGCXQSMw2SwcvuL+MIK7c9u9qsS3m5M9499S?=
 =?us-ascii?Q?UaJx8b4Oeg/bDXgjz7YNsMlNgUbO+Y6XIzs2CMbCU/IniScXMZykDuUc359v?=
 =?us-ascii?Q?yCa9RPOPirdvUvhSFumm0tyTQUl3OSRt8X9FojlwpstwrO2VTmUD7kPsYmtH?=
 =?us-ascii?Q?NEh4qG8rkRkCbvEUBhyKsIbYCL+8F9zubxnz3kkUFB6rSd9lud4TduLN6dGv?=
 =?us-ascii?Q?irEFoECxM1pnnyNwEUuzBxHK6RJ013ETQbUleXiK72w7vaIM/yrf4/SAX8i0?=
 =?us-ascii?Q?n3TubkInH2BdnpTToAyiFYgktUTCpPzJB9tFAvEF07C5z0zb8ojlpK7q4N6p?=
 =?us-ascii?Q?1r8wGbbSQa8n0LcV+mx41sNnBYybn0kUithiguHVglmbtChIGEqV2eQjEe8+?=
 =?us-ascii?Q?Q8aFmzqn3IiDtgBtFhgCLalKKxXrTjxVjOazNOgUmycDonm+aR1M7wchjXRx?=
 =?us-ascii?Q?LSLDynjbRrifW4/hYYbZdssJ6aW8C6Xz0B9PH+3Gy21/oiU9VYl5IWLtuGst?=
 =?us-ascii?Q?AwofljgL5t7p2dzcnt94UFUCgMdL2JBka5Sp8INm7yh1sPx/jTMHb6pJL3ur?=
 =?us-ascii?Q?wOQUWtLbkF7deMR7oxyxOLaofQa1Bae2YE3U+itPQvtgZjn2DNXx4NTXqYln?=
 =?us-ascii?Q?UxpWG25rqYY4rxNGDJNvyJZNvxA2H0rDNd/GA+SGETobagTo714vp3KD+MNc?=
 =?us-ascii?Q?nuTjO0PC3zzB8BFhhFzN7Y75MD6KR3Nqdig1y3BjDcv/R7GH7q0qTQLsQpDk?=
 =?us-ascii?Q?UZQNyddPlDohn0jhkGyebuILMEDCgf4gnMfF0OvfuTlKj2XlvO51sXSgZNfG?=
 =?us-ascii?Q?uetUs/g51Ng+hkwzL5SomD3x7/QMxIyrQuTKtY4SSRxK085kcQvWtoTF68J/?=
 =?us-ascii?Q?9PnldwYwbPTZvHsqBc/YCrmU77PqRMayAErOuQ=3D=3D?=
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?7TVwGPQJbkp+8AIgIPtncEPsPstEMJqp0x/rC1K2IZlIx1VSmIs9HS7m1RJL?=
 =?us-ascii?Q?9sd7BykW8IHoH2uH2sI2fwBe9K0AoHUIO18gwWuybiKkljKCNprigNACVD4Q?=
 =?us-ascii?Q?hKZXI0Xrm3UvlwB7dMZG81OcPpc6g+TKwX82BtoRlDvp7QjVWqKgo/Vkld7R?=
 =?us-ascii?Q?+QaI2TW2KtBEji5v0jbd5KC5ykd8Be5jEAQdRN7fttlWMIzv0tQRmJtz2sda?=
 =?us-ascii?Q?fr6butA6jlqq2bCgZAXcOr2YeE6D5aC9PeCUxDDtvxSj0vZf5+2Qx/0OfeKh?=
 =?us-ascii?Q?jjuhiOAEHT9A+wnWqZYOD4q8vkAY1UhujOQZzjlmR1sV1gwv+4T+DqvMdy6D?=
 =?us-ascii?Q?d0GZEXE9eQkX3j+M5XXzJmduCZm39apnpl9yJB+4Eb8ASHxcw14bfG9BXz3u?=
 =?us-ascii?Q?1RlgvtQ6nTc4Lr1Ngw4Y45OT7UZAiNnPShQSPyu9L+HF+BgIgtAmqWU8gmQG?=
 =?us-ascii?Q?ucDAA7UVu5iI+WAIbqjbSoOPlRx7qUPysPjrrs9iuXAnZDQGqhPqkwrFdfDa?=
 =?us-ascii?Q?MZFDTWSiARZfDFSIlhKeWkWIYEZbT8qdCBkTjVosGo3P/ut6VwScTAWyEnHT?=
 =?us-ascii?Q?6c+YrOIew91dVQT8Kd6mLSSVvhY5Sd2NIDRqU1vAiOuK8V7ZwpG0KzMM+N0r?=
 =?us-ascii?Q?NmBwcU6F7eYpgsob8uG5pajLMopq1dKL9NN877D8tkVogoIOlVyk1pVfGzTd?=
 =?us-ascii?Q?cBTDki7oiM0SmrjRk6sYlYVKvM0uQplUxJCUvJa3NX+wfnUwyE+GowEu7xug?=
 =?us-ascii?Q?lmKJeskW9on+2oSEYpr9NWp4NJ89zpysHnNzcMXRnM6pM7TZFoGumw4AvM9/?=
 =?us-ascii?Q?9mQyjnu6KrWh6r+Wd5zeU9vUTwx66Ou8LU2HS2g+x8nm8tcNGzwEA9s74nRn?=
 =?us-ascii?Q?QGMHxEAiKLvKvwE4IXvxDVZ8IpPdPxJxcFIEOnuAOy7kPI+5pUP+UdEeSnv6?=
 =?us-ascii?Q?wft43Jcg3HH0YDBftR04Z88UX0nuq468BEGrm0zH5ZLYZLRr/cwMf/Z3ZMnw?=
 =?us-ascii?Q?i/eHQVnI5HpY55gCDc1NCCzYg/CV24VNMQy/kU2kfFMAiTwd27BbKbaSrGaK?=
 =?us-ascii?Q?1+48cTWIXPy+3G2yfsJE4O7J0W6AoHt1ZYI64vjROXr4vp3mDAkjzrHjnhUU?=
 =?us-ascii?Q?ySEIBX/8nXve2jiM0JWDVFRH1MDdqqWuztfDVqCctpiorSeaDkyRBzp+ZPRH?=
 =?us-ascii?Q?sV6btHAqw5iwyOosPKGj9JBzark26s4Jvo1dj2czlDHBYCO3vYoQoJX1QsxT?=
 =?us-ascii?Q?nJsBEs3F3JXfWiQ1LFpJ?=
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e5faddde-01e7-463f-25a4-08dd658d3923
X-MS-Exchange-CrossTenant-AuthSource: CH3PR84MB3424.NAMPRD84.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Mar 2025 19:52:18.5970 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR84MB3282
Received-SPF: pass client-ip=40.92.44.61;
 envelope-from=Morgan.J.Smith@HIDDEN;
 helo=NAM02-SN1-obe.outbound.protection.outlook.com
X-Spam_score_int: -20
X-Spam_score: -2.1
X-Spam_bar: --
X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
 DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001,
 RCVD_IN_MSPIKE_H2=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001,
 RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_PASS=-0.001,
 SPF_PASS=-0.001 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-Spam-Score: 1.0 (+)
X-Debbugs-Envelope-To: submit
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -0.0 (/)

--=-=-=
Content-Type: text/plain

Tags: patch

This patch just adds some tests so I don't think that should be too
controversial

I have included some comments in the patch that I would like to talk
about though.

In the test `em-hist-test/eshell-history-file-name/nil' we don't save
any history to a file because `eshell-history-file-name' is nil and the
environment variable HISTFILE is unset.  I would like to see some user
visible warning here.  In theory this could be intended behavior to
prevent any history from being saved to disk but I think the more likely
case is the user accidentally misconfigured it.


I added a comment to the existing test
`em-hist-test/write-history/overwrite-multiple-shells' as I don't like
the behavior it demonstrates.  I was experiencing the occasional history
loss for years before I finally decided to investigate and now I know to
set `eshell-history-append' to true.

While writing this email I discovered Bug#66700 which seemed to come to
the conclusion that we should set `eshell-history-append' to true in the
next version of Emacs.  However, this would effectively make
`eshell-hist-ignoredups' useless.  I would be interested in a solution
where we check if the histfile has been updated to make sure we
overwrite without history loss.


--=-=-=
Content-Type: text/patch
Content-Disposition: attachment;
 filename=0001-Add-some-more-Eshell-history-tests.patch

From 3036053cc1ce861cc926efa6fe1ca544a0eb6621 Mon Sep 17 00:00:00 2001
From: Morgan Smith <Morgan.J.Smith@HIDDEN>
Date: Sun, 16 Mar 2025 15:06:44 -0400
Subject: [PATCH] Add some more Eshell history tests

* test/lisp/eshell/em-hist-tests.el
(em-hist-test/eshell-history-file-name/nil)
(em-hist-test/eshell-history-file-name/env)
(em-hist-test/eshell-history-size/nil)
(em-hist-test/eshell-history-size): New tests.
(em-hist-test/write-history/overwrite-multiple-shells): Add comment.
---
 test/lisp/eshell/em-hist-tests.el | 64 +++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/test/lisp/eshell/em-hist-tests.el b/test/lisp/eshell/em-hist-tests.el
index 6288ae071dc..71c08c99e02 100644
--- a/test/lisp/eshell/em-hist-tests.el
+++ b/test/lisp/eshell/em-hist-tests.el
@@ -51,6 +51,65 @@ em-hist-test/check-history-file
 
 ;;; Tests:
 
+(ert-deftest em-hist-test/eshell-history-file-name/nil ()
+  "Test setting 'history-file-name' and environment variable 'HISTFILE' to nil."
+  ;; It might be nice to warn the user in this case!  That would have
+  ;; saved me time debugging my init file.
+  (with-temp-eshell
+    (eshell-insert-command "echo hi")
+    (eshell-insert-command "echo bye")
+    (eshell-save-some-history) ; ensure no error
+    (should (equal (nreverse (ring-elements eshell-history-ring))
+                   '("echo hi" "echo bye")))))
+
+(ert-deftest em-hist-test/eshell-history-file-name/env ()
+  "Test setting history file from the environment variable 'HISTFILE'."
+  (ert-with-temp-file histfile
+    ;; A hack to set 'HISTFILE' as the test infrastructure clears it
+    (let ((buffer-list-update-hook
+           (list (lambda ()
+                   (setq process-environment
+                         (cons (concat "HISTFILE=" histfile)
+                               process-environment))))))
+      (with-temp-eshell
+        (em-hist-test/check-history-file histfile nil)
+        (eshell-insert-command "echo hi")
+        (eshell-save-some-history)
+        (em-hist-test/check-history-file histfile '("echo hi"))
+        (eshell-insert-command "echo bye")
+        (eshell-save-some-history)
+        (em-hist-test/check-history-file histfile '("echo hi" "echo bye"))))))
+
+(ert-deftest em-hist-test/eshell-history-size/nil ()
+  "Test setting `eshell-history-size' to nil.
+We should fallback to the default value."
+  (let ((process-environment (cons "HISTSIZE" process-environment))
+        (eshell-history-size nil))
+    (with-temp-eshell
+      (should
+       (equal eshell-history-size
+              (eval (car (get 'eshell-history-size 'standard-value)) t))))))
+
+(ert-deftest em-hist-test/eshell-history-size ()
+  "Test setting `eshell-history-size'."
+  (ert-with-temp-file histfile
+    (let ((process-environment (cons "HISTSIZE" process-environment))
+          (eshell-history-size 1))
+      (with-temp-eshell
+        (em-hist-test/check-history-file histfile nil)
+        (eshell-insert-command "echo hi")
+        (eshell-write-history histfile 'append)
+        (em-hist-test/check-history-file histfile '("echo hi"))
+        (eshell-insert-command "echo bye")
+        (eshell-write-history histfile 'append)
+        (em-hist-test/check-history-file histfile
+                                         '("echo hi" "echo bye")
+                                         '("echo bye"))
+        (eshell-insert-command "echo lost history")
+        (eshell-write-history histfile)
+        (em-hist-test/check-history-file histfile
+                                         '("echo lost history"))))))
+
 (ert-deftest em-hist-test/write-history/append ()
   "Test appending new history to history file."
   (ert-with-temp-file histfile
@@ -103,6 +162,11 @@ em-hist-test/write-history/overwrite
 
 (ert-deftest em-hist-test/write-history/overwrite-multiple-shells ()
   "Test overwriting history file from multiple Eshells."
+  ;; Is this really the behavior we want?  This is how eshell comes out
+  ;; of the box with zero configuration.  Users using multiple eshells
+  ;; are going to be missing history in a way that is going to be hard
+  ;; for them to track down as it depends on when shells are opened and
+  ;; closed relative to each other.
   (ert-with-temp-file histfile
     (with-temp-eshell
      (with-temp-eshell
-- 
2.48.1


--=-=-=--