* [PATCH] testing: Delete duplicate tests
@ 2023-07-12 19:22 Ilya Chernyshov
2023-07-13 9:51 ` Ihor Radchenko
2023-07-14 11:50 ` Max Nikulin
0 siblings, 2 replies; 10+ messages in thread
From: Ilya Chernyshov @ 2023-07-12 19:22 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 1102 bytes --]
Hello.
In my last patch, I found a duplicate test, so I decided to find all of
the duplicate tests inside testing/lisp/ folder via this function:
(defun count-duplicate-tests (&optional directory)
(let (files)
(dolist (file (directory-files (or directory default-directory) t (rx ".el" string-end) t))
(with-current-buffer (find-file-noselect file)
(save-excursion
(goto-char (point-min))
(while (search-forward "(ert-deftest" nil t)
(ignore-errors
(while-let((form (or (read (current-buffer)) t)))
(when (eq (car-safe form) 'should)
(setf
(alist-get form (alist-get file files nil nil #'equal) 0 nil #'equal)
(1+ (alist-get form (alist-get file files nil nil #'equal) 0 nil #'equal))))))))))
(seq-remove
(lambda(file) (null (cdr file)))
(mapcar
(lambda(file)
(cons
(car file)
(seq-filter
(lambda(form) (/= (cdr form) 1))
(cdr file))))
files))))
(setq dups (count-duplicate-tests "~/org-mode/testing/lisp/"))
Then I checked the result manually and deleted some of them.
Here is the patch I wrote:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-testing-Delete-duplicate-tests.patch --]
[-- Type: text/x-patch, Size: 4978 bytes --]
From 21ba128bd648c6737ed088abdd2a1824cfe01759 Mon Sep 17 00:00:00 2001
From: Ilya Chernyshov <ichernyshovvv@gmail.com>
Date: Thu, 13 Jul 2023 01:36:33 +0700
Subject: [PATCH] testing: Delete duplicate tests
* testing/lisp/test-ol.el (test-org-link/store-link): Delete a duplicate test.
* testing/lisp/test-org-clock.el (test-org-clock/clocktable/properties): Delete a duplicate test.
* testing/lisp/test-org-element.el (test-org-element/link-parser,
test-org-element/timestamp-parser): Delete duplicate tests.
* testing/lisp/test-org-table.el (test-org-table/get-field): Delete a duplicate test.
* testing/lisp/test-org.el (test-org/auto-fill-function): Delete a duplicate test.
---
testing/lisp/test-ol.el | 8 --------
testing/lisp/test-org-clock.el | 15 ---------------
testing/lisp/test-org-element.el | 10 ----------
testing/lisp/test-org-table.el | 4 ----
testing/lisp/test-org.el | 8 --------
5 files changed, 45 deletions(-)
diff --git a/testing/lisp/test-ol.el b/testing/lisp/test-ol.el
index a38d9f979..70be03818 100644
--- a/testing/lisp/test-ol.el
+++ b/testing/lisp/test-ol.el
@@ -301,14 +301,6 @@ See https://github.com/yantar92/org/issues/4."
(let ((file (buffer-file-name)))
(equal (format "[[file:%s::two]]" file file)
(org-store-link nil))))))
- (should
- (let ((org-stored-links nil)
- (org-context-in-file-links t))
- (org-test-with-temp-text-in-file "# two"
- (fundamental-mode)
- (let ((file (buffer-file-name)))
- (equal (format "[[file:%s::two]]" file file)
- (org-store-link nil))))))
(should
(let ((org-stored-links nil)
(org-context-in-file-links t))
diff --git a/testing/lisp/test-org-clock.el b/testing/lisp/test-org-clock.el
index d40939eb6..16cfc63a5 100644
--- a/testing/lisp/test-org-clock.el
+++ b/testing/lisp/test-org-clock.el
@@ -821,21 +821,6 @@ CLOCK: [2016-12-27 Wed 13:09]--[2016-12-28 Wed 15:09] => 26:00
:PROPERTIES:
:A: 1
:END:
-CLOCK: [2016-12-27 Wed 13:09]--[2016-12-28 Wed 15:09] => 26:00"
- (test-org-clock-clocktable-contents ":properties (\"A\")"))))
- ;; Handle missing properties.
- (should
- (equal
- "| A | Headline | Time |
-|---+--------------+---------|
-| | *Total time* | *26:00* |
-|---+--------------+---------|
-| 1 | Foo | 26:00 |"
- (org-test-with-temp-text
- "* Foo
-:PROPERTIES:
-:A: 1
-:END:
CLOCK: [2016-12-27 Wed 13:09]--[2016-12-28 Wed 15:09] => 26:00"
(test-org-clock-clocktable-contents ":properties (\"A\")")))))
diff --git a/testing/lisp/test-org-element.el b/testing/lisp/test-org-element.el
index 2e3a249ab..d95195f0d 100644
--- a/testing/lisp/test-org-element.el
+++ b/testing/lisp/test-org-element.el
@@ -2436,11 +2436,6 @@ e^{i\\pi}+1=0
(let ((file (expand-file-name (buffer-file-name))))
(insert (format "[[file:%s]]" file))
(equal (org-element-property :path (org-element-context)) file))))
- (should
- (org-test-with-temp-text-in-file ""
- (let ((file (expand-file-name (buffer-file-name))))
- (insert (format "[[file:%s]]" file))
- (equal (org-element-property :path (org-element-context)) file))))
;; ... multi-line link.
(should
(equal "ls *.org"
@@ -3195,11 +3190,6 @@ Outside list"
(org-test-with-temp-text "<2023-07-02 Sun 12:00>--<2023-07-02 Sun 13:00>"
(org-element-property :range-type (org-element-timestamp-parser)))
'daterange))
- (should
- (eq
- (org-test-with-temp-text "<2023-07-02 Sun 12:00>--<2023-07-02 Sun>"
- (org-element-property :range-type (org-element-timestamp-parser)))
- 'daterange))
(should
(eq
(org-test-with-temp-text "<2023-07-02 Sun 12:00 +5d>--<2023-07-02 Sun 13:00>"
diff --git a/testing/lisp/test-org-table.el b/testing/lisp/test-org-table.el
index 27aeb5ab3..ce78d4488 100644
--- a/testing/lisp/test-org-table.el
+++ b/testing/lisp/test-org-table.el
@@ -3368,10 +3368,6 @@ See also `test-org-table/copy-field'."
(org-test-with-temp-text "| 1 | 2 | 3 |"
(org-table-get-field 3 " foo ")
(buffer-string))))
- (should
- (equal " 4 "
- (org-test-with-temp-text "| 1 | 2 |\n<point>| 3 | 4 |"
- (org-table-get-field 2))))
;; An empty REPLACE string clears the field.
(should
(equal "| |"
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 08cda543a..47dd9b876 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -994,14 +994,6 @@
(org-auto-fill-function)
(buffer-string)))))
;; Comment block: auto fill contents.
- (should
- (equal "#+BEGIN_COMMENT\n12345\n7890\n#+END_COMMENT"
- (org-test-with-temp-text "#+BEGIN_COMMENT\n12345 7890\n#+END_COMMENT"
- (let ((fill-column 5))
- (forward-line)
- (end-of-line)
- (org-auto-fill-function)
- (buffer-string)))))
(should
(equal "#+BEGIN_COMMENT\n12345\n7890\n#+END_COMMENT"
(org-test-with-temp-text "#+BEGIN_COMMENT\n12345 7890\n#+END_COMMENT"
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] testing: Delete duplicate tests
2023-07-12 19:22 [PATCH] testing: Delete duplicate tests Ilya Chernyshov
@ 2023-07-13 9:51 ` Ihor Radchenko
2023-08-08 12:44 ` Ihor Radchenko
2023-07-14 11:50 ` Max Nikulin
1 sibling, 1 reply; 10+ messages in thread
From: Ihor Radchenko @ 2023-07-13 9:51 UTC (permalink / raw)
To: Ilya Chernyshov; +Cc: emacs-orgmode
Ilya Chernyshov <ichernyshovvv@gmail.com> writes:
> In my last patch, I found a duplicate test, so I decided to find all of
> the duplicate tests inside testing/lisp/ folder via this function:
Thanks!
> --- a/testing/lisp/test-org-table.el
> +++ b/testing/lisp/test-org-table.el
> @@ -3368,10 +3368,6 @@ See also `test-org-table/copy-field'."
> (org-test-with-temp-text "| 1 | 2 | 3 |"
> (org-table-get-field 3 " foo ")
> (buffer-string))))
> - (should
> - (equal " 4 "
> - (org-test-with-temp-text "| 1 | 2 |\n<point>| 3 | 4 |"
> - (org-table-get-field 2))))
It looks like the real test is supposed to be
(equal " foo "
...
(org-table-get-field 2 " foo ")
(buffer-string)
> --- a/testing/lisp/test-org.el
> +++ b/testing/lisp/test-org.el
> @@ -994,14 +994,6 @@
> (org-auto-fill-function)
> (buffer-string)))))
> ;; Comment block: auto fill contents.
> - (should
> - (equal "#+BEGIN_COMMENT\n12345\n7890\n#+END_COMMENT"
> - (org-test-with-temp-text "#+BEGIN_COMMENT\n12345 7890\n#+END_COMMENT"
> - (let ((fill-column 5))
> - (forward-line)
> - (end-of-line)
> - (org-auto-fill-function)
> - (buffer-string)))))
> (should
> (equal "#+BEGIN_COMMENT\n12345\n7890\n#+END_COMMENT"
> (org-test-with-temp-text "#+BEGIN_COMMENT\n12345 7890\n#+END_COMMENT"
And this is the result of 8a97c60
Do not fill verse blocks contents
* lisp/org.el (org-fill-context-prefix, org-fill-paragraph): Do not
fill verse blocks contents. Verse blocks can be used to format
free-form poetry, so filling has to be done manually.
* testing/lisp/test-org.el: Remove unnecessary tests.
The test was changed from testing verse block into a duplicate.
I think the right thing to do here would be `should-not' + old version
of the test with VERSE block.
--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] testing: Delete duplicate tests
2023-07-12 19:22 [PATCH] testing: Delete duplicate tests Ilya Chernyshov
2023-07-13 9:51 ` Ihor Radchenko
@ 2023-07-14 11:50 ` Max Nikulin
2023-07-15 7:56 ` Ihor Radchenko
1 sibling, 1 reply; 10+ messages in thread
From: Max Nikulin @ 2023-07-14 11:50 UTC (permalink / raw)
To: Ilya Chernyshov, emacs-orgmode
On 13/07/2023 02:22, Ilya Chernyshov wrote:
> +++ b/testing/lisp/test-ol.el
> @@ -301,14 +301,6 @@ Seehttps://github.com/yantar92/org/issues/4."
> (let ((file (buffer-file-name)))
> (equal (format "[[file:%s::two]]" file file)
> (org-store-link nil))))))
> - (should
> - (let ((org-stored-links nil)
> - (org-context-in-file-links t))
> - (org-test-with-temp-text-in-file "# two"
> - (fundamental-mode)
> - (let ((file (buffer-file-name)))
> - (equal (format "[[file:%s::two]]" file file)
> - (org-store-link nil))))))
The test was added by
7a78eb1be 2020-03-26 22:57:16 +0100 Nicolas Goaziou: ol: Fix some corner
cases when normalizing context in links
The intention may be to test "#two" besides "# two". Maybe somebody has
a better guess what case related to the change is not covered.
The idea to find duplicated tests is bright. The code may be transformed
in a dedicated unit test. I would not drop existing tests blindly though.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] testing: Delete duplicate tests
2023-07-14 11:50 ` Max Nikulin
@ 2023-07-15 7:56 ` Ihor Radchenko
0 siblings, 0 replies; 10+ messages in thread
From: Ihor Radchenko @ 2023-07-15 7:56 UTC (permalink / raw)
To: Max Nikulin; +Cc: Ilya Chernyshov, emacs-orgmode
Max Nikulin <manikulin@gmail.com> writes:
> The idea to find duplicated tests is bright. The code may be transformed
> in a dedicated unit test. I would not drop existing tests blindly though.
Very good idea indeed.
One gotcha is that not every should form is next level after
ert-deftest. And there are also "should-not" and "should-error" forms.
So, we may compare only forms at the same sexp depth for equality,
without restricting to "should".
--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] testing: Delete duplicate tests
2023-07-13 9:51 ` Ihor Radchenko
@ 2023-08-08 12:44 ` Ihor Radchenko
2023-08-31 6:17 ` Ilya Chernyshov
0 siblings, 1 reply; 10+ messages in thread
From: Ihor Radchenko @ 2023-08-08 12:44 UTC (permalink / raw)
To: Ilya Chernyshov; +Cc: emacs-orgmode
Ihor Radchenko <yantar92@posteo.net> writes:
> It looks like the real test is supposed to be
>
> (equal " foo "
> ...
> (org-table-get-field 2 " foo ")
> (buffer-string)
> ...
I have re-introduced the new tests in place of the removed ones,
according to my and Max's findings. On top of the patch.
Applied, onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=fe85d61a9
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=173b5de0e
--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] testing: Delete duplicate tests
2023-08-08 12:44 ` Ihor Radchenko
@ 2023-08-31 6:17 ` Ilya Chernyshov
2023-08-31 6:29 ` Ihor Radchenko
0 siblings, 1 reply; 10+ messages in thread
From: Ilya Chernyshov @ 2023-08-31 6:17 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
Ihor Radchenko <yantar92@posteo.net> writes:
> I have re-introduced the new tests in place of the removed ones,
> according to my and Max's findings. On top of the patch.
>
> Applied, onto main.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=fe85d61a9
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=173b5de0e
Thank you! If a function that checks for duplicate tests is a welcome
addition to org tests, I'll send is as a patch. What do you think?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] testing: Delete duplicate tests
2023-08-31 6:17 ` Ilya Chernyshov
@ 2023-08-31 6:29 ` Ihor Radchenko
2023-11-08 9:59 ` Ihor Radchenko
0 siblings, 1 reply; 10+ messages in thread
From: Ihor Radchenko @ 2023-08-31 6:29 UTC (permalink / raw)
To: Ilya Chernyshov; +Cc: emacs-orgmode
Ilya Chernyshov <ichernyshovvv@gmail.com> writes:
> Thank you! If a function that checks for duplicate tests is a welcome
> addition to org tests, I'll send is as a patch. What do you think?
It would be great. Thanks in advance!
--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] testing: Delete duplicate tests
2023-08-31 6:29 ` Ihor Radchenko
@ 2023-11-08 9:59 ` Ihor Radchenko
2023-11-11 8:55 ` Ilya Chernyshov
0 siblings, 1 reply; 10+ messages in thread
From: Ihor Radchenko @ 2023-11-08 9:59 UTC (permalink / raw)
To: Ilya Chernyshov; +Cc: emacs-orgmode
Ihor Radchenko <yantar92@posteo.net> writes:
> Ilya Chernyshov <ichernyshovvv@gmail.com> writes:
>
>> Thank you! If a function that checks for duplicate tests is a welcome
>> addition to org tests, I'll send is as a patch. What do you think?
>
> It would be great. Thanks in advance!
I saw you using your function to detect the existing duplicate tests.
However, it would also be nice to add it as a test of its own to detect
duplicates in future. WDYT?
--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] testing: Delete duplicate tests
2023-11-08 9:59 ` Ihor Radchenko
@ 2023-11-11 8:55 ` Ilya Chernyshov
2023-11-16 12:27 ` Ilya Chernyshov
0 siblings, 1 reply; 10+ messages in thread
From: Ilya Chernyshov @ 2023-11-11 8:55 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 713 bytes --]
Ihor Radchenko <yantar92@posteo.net> writes:
> I saw you using your function to detect the existing duplicate tests.
> However, it would also be nice to add it as a test of its own to detect
> duplicates in future. WDYT?
Sure, here it is. In the patch, I added a new file
(testing/lisp/test-deduplicator.el) with a test that checks for
duplicate forms (not just should, should-not, should-error macros) in
all test files.
Changes in other files serve as an example of how to use
`org-test-ignore-duplicate' to make sure that the test deduplicator
skips certain duplicate forms.
There's a lot of tests to change before merging. I'll handle them and
submit a new patch if you have no questions about the code.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-testing-Add-testing-lisp-test-deduplicator.el.patch --]
[-- Type: text/x-patch, Size: 15636 bytes --]
From 3b38450f7de8bd168d8795728454d9f4db720843 Mon Sep 17 00:00:00 2001
From: Ilya Chernyshov <ichernyshovvv@gmail.com>
Date: Tue, 5 Sep 2023 22:40:59 +0700
Subject: [PATCH] testing: Add testing/lisp/test-deduplicator.el
* testing/lisp/test-deduplicator.el: Add test unit that checks for
duplicate forms in ert tests.
* testing/lisp/test-ob-lob.el (test-ob-lob/caching-call-line,
test-ob-lob/named-caching-call-line, test-ob/just-one-results-block):
Ignore duplicate forms via `org-test-ignore-duplicate'
* testing/lisp/test-ob.el (test-ob/just-one-results-block): Ignore
duplicate forms via `org-test-ignore-duplicate'
* testing/lisp/test-org.el (test-org/goto-sibling,
test-org/backward-element, test-org/up-element): Ignore duplicate
forms via `org-test-ignore-duplicate'
---
testing/lisp/test-deduplicator.el | 224 ++++++++++++++++++++++++++++++
testing/lisp/test-ob-lob.el | 10 +-
testing/lisp/test-ob.el | 3 +-
testing/lisp/test-org.el | 81 ++++++-----
4 files changed, 275 insertions(+), 43 deletions(-)
create mode 100644 testing/lisp/test-deduplicator.el
diff --git a/testing/lisp/test-deduplicator.el b/testing/lisp/test-deduplicator.el
new file mode 100644
index 000000000..28b5d66f0
--- /dev/null
+++ b/testing/lisp/test-deduplicator.el
@@ -0,0 +1,224 @@
+;;; test-deduplicator.el --- Tests for finding duplicates in Org tests -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2023 Ilya Chernyshov
+;; Authors: Ilya Chernyshov <ichernyshovvv@gmail.com>
+
+;; This file is not part of GNU Emacs.
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program. If not, see <https://www.gnu.org/licenses/>.
+;;
+;;; Commentary:
+
+;; Unit tests that check for duplicate forms (including `should',
+;; `should-not', `should-error') in all Org test files. Forms are
+;; considered duplicate if they are `equal-including-properties' and
+;; nested at the same level. To ignore a form or a group of forms,
+;; wrap them in `org-test-ignore-duplicate'.
+
+;;; Code:
+
+(require 'org-test "../testing/org-test")
+
+(defvar test-deduplicator-files
+ (directory-files (expand-file-name "lisp" org-test-dir) t "\\.el$"))
+
+(defvar test-deduplicator-duplicate-forms nil
+ "A nested list of the form:
+
+ (((file test-name [(form-1 . numerical-order)
+ (form-2 . numerical-order) ...])
+ (dup-form-1 . (numerical-order [numerical-order ...]))
+ [ (dup-form-2 . (numerical-order [numerical-order ...]))
+ (dup-form-3 . (numerical-order [numerical-order ...]))
+ ...])
+
+ ((file test-name [(form-1 . numerical-order)
+ (form-2 . numerical-order) ...])
+ (dup-form-1 . (numerical-order [numerical-order ...]))
+ [ (dup-form-2 . (numerical-order [numerical-order ...]))
+ (dup-form-3 . (numerical-order [numerical-order ...]))
+ ...])
+
+ ...
+ )
+
+Where
+
+ (file test-name [(form-1 . numerical-order)
+ (form-2 . numerical-order) ...])
+
+is a path to duplicates. For example, the path for the
+duplicates in the following test:
+
+ test-ob-haskell-ghci.el
+
+ (ertdeftest ob-haskell/session-named-none-means-one-shot-sessions ()
+ \"When no session, use a new session.
+ \"none\" is a special name that means `no session'.\"
+ (let ((var-1 \"value\"))
+ (when var-1
+ (should-not (equal 2 (test-ob-haskell-ghci \":session \"none\"\" \"x\" nil)))
+ (test-ob-haskell-ghci \":session none\" \"x=2\")
+ (should-not (equal 2 (test-ob-haskell-ghci \":session \"none\"\" \"x\" nil)))
+ (test-ob-haskell-ghci \":session none\" \"x=2\"))))
+
+would look like this:
+
+ (\"test-ob-haskell-ghci.el\"
+ ob-haskell/session-named-none-means-one-shot-sessions
+ (let . 4) (when . 2))
+
+And the records about the duplicates would look like this:
+
+ ((test-ob-haskell-ghci \":session none\" \"x=2\") 5 3)
+ ((should-not (equal 2 (test-ob-haskell-ghci \":session \"none\"\" \"x\" nil))) 4 2)")
+
+(defvar test-deduplicator-forms nil
+ "Nested alist of found forms and paths to them (not filtered).")
+
+(defmacro org-test-ignore-duplicate (&rest body)
+ "Eval BODY forms sequentially and return value of last one.
+
+The macro's body will be ignored by
+`test-deduplicator/detect-duplicate-tests' test to ignore
+duplicate forms inside the body."
+ (declare (indent 0))
+ `(progn ,@body))
+
+(ert-deftest test-org-tests/detect-duplicate-tests ()
+ "Try to find duplicate forms."
+
+ (should-not (test-deduplicator-find-duplicates test-deduplicator-files)))
+
+(defun test-deduplicator-find-duplicates (files)
+ "Try to find duplicate forms in FILES.
+
+If duplicates are found, record them into
+`test-deduplicator-duplicate-forms', `message' paths to them in a
+human-readable format and return the value.
+
+Forms are considered duplicate if they are nested at the same
+level."
+ (setq test-deduplicator-forms nil)
+ (dolist (file files)
+ (with-current-buffer (find-file-noselect file)
+ (save-excursion
+ (goto-char (point-min))
+ (while (search-forward "(ert-deftest" nil t)
+ (goto-char (match-beginning 0))
+ (ignore-errors
+ (while-let ((form (or (read (current-buffer)) t)))
+ (test-deduplicator-search-forms-recursively
+ form (list file (cadr form)))))))))
+ (setq test-deduplicator-duplicate-forms
+ (seq-filter
+ #'cdr (mapcar
+ (lambda (file)
+ (cons
+ (car file)
+ (seq-filter #'caddr (cdr file))))
+ test-deduplicator-forms)))
+ (when test-deduplicator-duplicate-forms
+ (let ((res (concat "Found duplicates (To ignore duplicate forms,\n"
+ "wrap them in `org-test-ignore-duplicate'):\n")))
+ (dolist (path test-deduplicator-duplicate-forms)
+ (let* ((file (file-relative-name (caar path)))
+ (test-name (symbol-name (cadar path)))
+ (path-inside-test (cddar path))
+ (result "")
+ (string-path (append (list file test-name)
+ (mapcar (lambda (x)
+ (symbol-name (car x)))
+ path-inside-test)))
+ (iter 0)
+ (print-level 3))
+ (dolist (x string-path)
+ (cl-callf concat result
+ (format "%s%s\n" (make-string (* iter 2) ? ) x))
+ (cl-incf iter))
+ (cl-callf concat result
+ (mapconcat
+ (lambda (x) (format "%s%S: %d times\n"
+ (make-string (* iter 2) ? )
+ (car x)
+ (length (cdr x))))
+ (cdr path)))
+ (cl-callf concat res result)))
+ (message "%s" res)))
+ test-deduplicator-duplicate-forms)
+
+(defun test-deduplicator-search-forms-recursively (form form-path)
+ "Search for forms recursively in FORM.
+
+FORM-PATH is list of the form:
+ (\"file-path\" ert-test-symbol
+ (symbol-1 . sexp-order-1) (symbol-2 . sexp-order-2))
+
+Write each form to `test-deduplicator-forms'"
+ (dotimes (iter (length form))
+ (pcase (car-safe (nth iter form))
+ ((or `skip-unless `org-test-ignore-duplicate))
+ ((pred (not null))
+ (push iter (alist-get (nth iter form)
+ (alist-get form-path test-deduplicator-forms
+ nil nil #'equal)
+ nil nil #'equal-including-properties))
+ (unless (member (car-safe (nth iter form))
+ '(should-not should should-error))
+ (test-deduplicator-search-forms-recursively
+ (nth iter form)
+ (append form-path (list (cons (car (nth iter form)) iter)))))))))
+
+;;; Tests
+
+(defvar test-deduplicator-file-path
+ (expand-file-name "test-deduplicator.el"
+ (expand-file-name "lisp" org-test-dir)))
+
+(ert-deftest test-org-tests/testing-test-deduplicator ()
+ ""
+ (should
+ (equal
+ (test-deduplicator-find-duplicates
+ (list test-deduplicator-file-path))
+ `(((,(expand-file-name "lisp/test-deduplicator.el" org-test-dir)
+ test-org-tests/test-with-nested-duplicates)
+ ((format "%s" "string") 7 5)
+ ((let ((var "string")) (should (message "123 %s" var))) 6 4))
+ (((expand-file-name "lisp/test-deduplicator.el" org-test-dir)
+ test-org-tests/test-with-duplicates-at-root)
+ ((should (message "123")) 6 4))))))
+
+;;; Tests with duplicate forms to check the deduplicator
+
+(ert-deftest test-org-tests/test-with-duplicates-at-root ()
+ "Test with duplicates at the root."
+ (should (message "123"))
+ (format "%s" "string")
+ (should
+ (message "123")))
+
+(ert-deftest test-org-tests/test-with-nested-duplicates ()
+ "Test with nested duplicates."
+ (let ((var "string"))
+ (should
+ (message "123 %s" var)))
+ (format "%s" "string")
+ (let ((var "string"))
+ (should (message "123 %s" var)))
+ (format "%s" "string"))
+
+(provide 'test-deduplicator)
+
+;;; test-deduplicator.el ends here
diff --git a/testing/lisp/test-ob-lob.el b/testing/lisp/test-ob-lob.el
index 188fee4c0..66dfd0eab 100644
--- a/testing/lisp/test-ob-lob.el
+++ b/testing/lisp/test-ob-lob.el
@@ -152,8 +152,9 @@ for export
(should
(eq (org-babel-execute-src-block nil (org-babel-lob-get-info)) 1))
;; if cached, second evaluation will retain the t value
- (should
- (eq (org-babel-execute-src-block nil (org-babel-lob-get-info)) 1)))))
+ (org-test-ignore-duplicate
+ (should
+ (eq (org-babel-execute-src-block nil (org-babel-lob-get-info)) 1))))))
(ert-deftest test-ob-lob/named-caching-call-line ()
(let ((temporary-value-for-test 0))
@@ -170,8 +171,9 @@ for export
(should
(eq (org-babel-execute-src-block nil (org-babel-lob-get-info)) 1))
;; if cached, second evaluation will retain the t value
- (should
- (eq (org-babel-execute-src-block nil (org-babel-lob-get-info)) 1)))))
+ (org-test-ignore-duplicate
+ (should
+ (eq (org-babel-execute-src-block nil (org-babel-lob-get-info)) 1))))))
(ert-deftest test-ob-lob/assignment-with-newline ()
"Test call lines with an argument containing a newline character."
diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el
index 42c77ca56..0153de889 100644
--- a/testing/lisp/test-ob.el
+++ b/testing/lisp/test-ob.el
@@ -645,7 +645,8 @@ duplicate results block."
(org-babel-execute-src-block)
(org-babel-execute-src-block) ; second code block execution
(should (search-forward "Hello")) ; the string inside the source code block
- (should (search-forward "Hello")) ; the same string in the results block
+ (org-test-ignore-duplicate
+ (should (search-forward "Hello"))) ; the same string in the results block
(should-error (search-forward "Hello"))))
(ert-deftest test-ob/nested-code-block ()
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 612bfa1e5..4e23488be 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -2490,7 +2490,8 @@ Text.
(should-not (org-goto-sibling))
(should (org-goto-sibling 'previous))
(should (looking-at-p "^\\*\\* Heading 2"))
- (should (org-goto-sibling 'previous))
+ (org-test-ignore-duplicate
+ (should (org-goto-sibling 'previous)))
(should (looking-at-p "^\\*\\* Heading 1"))
(should-not (org-goto-sibling 'previous)))
;; Inside heading.
@@ -2533,7 +2534,8 @@ test <point>
(should-not (org-goto-sibling))
(should (org-goto-sibling 'previous))
(should (looking-at-p "^\\*\\* Heading 2"))
- (should (org-goto-sibling 'previous))
+ (org-test-ignore-duplicate
+ (should (org-goto-sibling 'previous)))
(should (looking-at-p "^\\*\\* Heading 1"))
(should-not (org-goto-sibling 'previous)))))
@@ -5223,27 +5225,28 @@ Outside."
;; 7.1. At beginning of sub-list: expected to move to the
;; paragraph before it.
(goto-line 4)
- (org-backward-element)
- (should (looking-at "item1"))
- ;; 7.2. At an item in a list: expected to move at previous item.
- (goto-line 8)
- (org-backward-element)
- (should (looking-at " - sub2"))
- (goto-line 12)
- (org-backward-element)
- (should (looking-at "- item1"))
- ;; 7.3. At end of list/sub-list: expected to move to list/sub-list
- ;; beginning.
- (goto-line 10)
- (org-backward-element)
- (should (looking-at " - sub1"))
- (goto-line 15)
- (org-backward-element)
- (should (looking-at "- item1"))
- ;; 7.4. At blank-lines before list end: expected to move to top
- ;; item.
- (goto-line 14)
- (org-backward-element)
+ (org-test-ignore-duplicate
+ (org-backward-element)
+ (should (looking-at "item1"))
+ ;; 7.2. At an item in a list: expected to move at previous item.
+ (goto-line 8)
+ (org-backward-element)
+ (should (looking-at " - sub2"))
+ (goto-line 12)
+ (org-backward-element)
+ (should (looking-at "- item1"))
+ ;; 7.3. At end of list/sub-list: expected to move to list/sub-list
+ ;; beginning.
+ (goto-line 10)
+ (org-backward-element)
+ (should (looking-at " - sub1"))
+ (goto-line 15)
+ (org-backward-element)
+ (should (looking-at "- item1"))
+ ;; 7.4. At blank-lines before list end: expected to move to top
+ ;; item.
+ (goto-line 14)
+ (org-backward-element))
(should (looking-at "- item1"))))
(ert-deftest test-org/up-element ()
@@ -5281,21 +5284,23 @@ Outside."
- item2"
;; 4.1. Within an item: move to the item beginning.
(goto-line 8)
- (org-up-element)
- (should (looking-at " - sub2"))
- ;; 4.2. At an item in a sub-list: move to parent item.
- (goto-line 4)
- (org-up-element)
- (should (looking-at "- item1"))
- ;; 4.3. At an item in top list: move to beginning of whole list.
- (goto-line 10)
- (org-up-element)
- (should (looking-at "- item1"))
- ;; 4.4. Special case. At very top point: should move to parent of
- ;; list.
- (goto-line 2)
- (org-up-element)
- (should (looking-at "\\* Top"))))
+ (org-test-ignore-duplicate
+ (org-up-element)
+ (should (looking-at " - sub2"))
+ ;; 4.2. At an item in a sub-list: move to parent item.
+ (goto-line 4)
+ (org-up-element)
+ (should (looking-at "- item1"))
+ ;; 4.3. At an item in top list: move to beginning of whole list.
+ (goto-line 10)
+ (org-up-element)
+ (org-test-ignore-duplicate
+ (should (looking-at "- item1")))
+ ;; 4.4. Special case. At very top point: should move to parent of
+ ;; list.
+ (goto-line 2)
+ (org-up-element)
+ (should (looking-at "\\* Top")))))
(ert-deftest test-org/down-element ()
"Test `org-down-element' specifications."
--
2.41.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] testing: Delete duplicate tests
2023-11-11 8:55 ` Ilya Chernyshov
@ 2023-11-16 12:27 ` Ilya Chernyshov
0 siblings, 0 replies; 10+ messages in thread
From: Ilya Chernyshov @ 2023-11-16 12:27 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
Ilya Chernyshov <ichernyshovvv@gmail.com> writes:
> Sure, here it is. In the patch, I added a new file
> (testing/lisp/test-deduplicator.el) with a test that checks for
> duplicate forms (not just should, should-not, should-error macros) in
> all test files.
It also makes sense to add a test that searches for completely equal
`ert-deftest' definitions and tests with equal bodies, but different
names, as you already advised on Matrix. Any other thoughts?
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-11-16 12:28 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-12 19:22 [PATCH] testing: Delete duplicate tests Ilya Chernyshov
2023-07-13 9:51 ` Ihor Radchenko
2023-08-08 12:44 ` Ihor Radchenko
2023-08-31 6:17 ` Ilya Chernyshov
2023-08-31 6:29 ` Ihor Radchenko
2023-11-08 9:59 ` Ihor Radchenko
2023-11-11 8:55 ` Ilya Chernyshov
2023-11-16 12:27 ` Ilya Chernyshov
2023-07-14 11:50 ` Max Nikulin
2023-07-15 7:56 ` Ihor Radchenko
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).