Re: [PATCH] testing: Delete duplicate tests

[Ihor Radchenko]

Ilya Chernyshov  writes:

> Yeah, now it works as it should. Thanks. I've made some minor changes
> I've described in the attached patch.

Thanks!
I squashed all the patches modifying the new test library into a single
commit and changed the duplicate in test-org/file-contents to achieve
its intended goal as stated in the commentary.
Applied, onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=8d2fcfea9
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=e3f327d1e
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=8e2ed45bb

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] testing: Delete duplicate tests

[Ilya Chernyshov]

Ihor Radchenko  writes:

> Note that your
> `test-org-tests/test-duplicates-detector-testing-find-duplicates' does
> not look right. I had to adjust the `equal' condition in order to make
> it pass. May you please check if the return value of
> `test-duplicates-detector--find-duplicates' is what you intended?

Yeah, now it works as it should. Thanks. I've made some minor changes
I've described in the attached patch.

>From 8bcd02bac32d3a4442814c2a42b097d642964372 Mon Sep 17 00:00:00 2001
From: Ilya Chernyshov 
Date: Fri, 9 Feb 2024 17:32:58 +0600
Subject: [PATCH] test-duplicates-detector.el: Simplify the docs, refactor,
 optimize the search

*
test-duplicates-detector.el (test-duplicates-detector-duplicate-forms):
Simplify the docstring.  Add that the list also may have information
about duplicate ert test definitions.

*
test-duplicates-detector.el (test-duplicates-detector--find-duplicates):
Don't go through a duplicate ert test definition.

*
test-duplicates-detector.el (test-duplicates-detector--search-forms-recursively):
Replace (car-safe sub-form) with (car sub-form) because we already
checked that sub-form is a cons.
---
 testing/lisp/test-duplicates-detector.el | 51 ++--
 1 file changed, 22 insertions(+), 29 deletions(-)

diff --git a/testing/lisp/test-duplicates-detector.el b/testing/lisp/test-duplicates-detector.el
index 25293f185..58da27c07 100644
--- a/testing/lisp/test-duplicates-detector.el
+++ b/testing/lisp/test-duplicates-detector.el
@@ -61,24 +61,19 @@ Immediate children inside these are not checked for duplicates.")
 (expand-file-name "lisp" org-test-dir) t "\\.el$")))
 
 (defvar test-duplicates-detector-duplicate-forms nil
-  "A nested list of the form:
+  "A list where each element is either:
 
-  (((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) ...])
+  ((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 ...]))
  ...])
 
-   ...
-  )
+or
+
+  (test-1-symbol . duplicate-of-test-1-symbol)
+
 
 Where
 
@@ -88,31 +83,29 @@ Where
 is a path to duplicates.  For example, the path for the
 duplicates in the following test:
 
- test-ob-haskell-ghci.el
+ test-file.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'.\"
+  (ertdeftest test-name ()
+\"Docstring.\"
 (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\")
+(equal 2 (some-func \"string\" \"x\" nil)))
+   (some-func \"string\" \"x=2\")
(should-not
-(equal 2 (test-ob-haskell-ghci \":session \"none\"\" \"x\" nil)))
-   (test-ob-haskell-ghci \":session none\" \"x=2\"
+(equal 2 (some-func \"string\" \"x\" nil)))
+   (some-func \"string\" \"x=2\"
 
 would look like this:
 
-  (\"test-ob-haskell-ghci.el\"
-ob-haskell/session-named-none-means-one-shot-sessions
+  (\"/absolute/path/to/test-file.el\"
+test-name
 (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)")
+(equal 2 (some-func \"string\" \"x\" nil))) 4 2)")
 
 (defvar test-duplicates-detector-forms nil
   "Nested alist of found forms and paths to them (not filtered).")
@@ -168,9 +161,9 @@ Duplicate forms will be written to
   (cdddr x
 			 found-deftests)))
 (push (cons test-name (cadr f)) duplicate-tests)
-		  (push deftest found-deftests))
-		(test-duplicates-detector--search-forms-recursively
-		 deftest (list file (cadr deftest)
+		  (push deftest found-deftests)
+  (test-duplicates-detector--search-forms-recursively
+		   deftest (list file test-name)
 (setq test-duplicates-detector-duplicate-forms
   (seq-filter
 	   #'cdr
@@ -239,11 +232,11 @@ Write each form to `test-duplicates-detector-forms'"
  (alist-get form-path test-duplicates-detector-forms
 nil nil #'equal)
  nil nil #'equal-including-properties)))
-(unless (memq (car-safe sub-form)
+ 

Re: [PATCH] testing: Delete duplicate tests

[Ihor Radchenko]

Ilya Chernyshov  writes:

> Ihor Radchenko  writes:
>
>> What about the attached amendment?
>> It should simplify things significantly.
>
> Sorry, in my previous patch the test that checks the detector itself was not
> even run in 'make test' because of incorrect test prefix.
>
> Your patch does not work as you expect. Could you please revise it?

Sure. I erroneously checked car of sub-form when testing whether to
ignore testing duplicates in children.

See the attached series of patches to be applied on top the previous 3.

Note that your
`test-org-tests/test-duplicates-detector-testing-find-duplicates' does
not look right. I had to adjust the `equal' condition in order to make
it pass. May you please check if the return value of
`test-duplicates-detector--find-duplicates' is what you intended?

>From 316c36d2b388a81e43fabf392a75af85532f56ba Mon Sep 17 00:00:00 2001
Message-ID: <316c36d2b388a81e43fabf392a75af85532f56ba.1706703212.git.yanta...@posteo.net>
From: Ihor Radchenko 
Date: Wed, 31 Jan 2024 13:11:31 +0100
Subject: [PATCH 1/3] testing/lisp/test-duplicates-detector.el: Fix recursion,
 add more ignored forms

*
testing/lisp/test-duplicates-detector.el (test-duplicates-detector--search-forms-recursively):
Do not test for duplicates according to parent form, not current form.

*
testing/lisp/test-duplicates-detector.el (test-duplicates-progn-forms):
Add more progn-like forms.
---
 testing/lisp/test-duplicates-detector.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/testing/lisp/test-duplicates-detector.el b/testing/lisp/test-duplicates-detector.el
index aed8034ee..b23fbce1e 100644
--- a/testing/lisp/test-duplicates-detector.el
+++ b/testing/lisp/test-duplicates-detector.el
@@ -42,9 +42,10 @@ (require 'org-test "../testing/org-test")
  Variables
 
 (defvar test-duplicates-progn-forms
-  '( progn
+  '( progn prog1 let dolist dotimes
  org-test-with-temp-text
  org-test-with-temp-text-in-file
+ org-test-at-id
  org-test-ignore-duplicate)
   "List of forms equivalent to `progn'.
 Immediate children inside these are not checked for duplicates.")
@@ -232,7 +233,7 @@ (defun test-duplicates-detector--search-forms-recursively (form form-path)
   (let ((idx 0))
 (dolist (sub-form form)
   (when (consp sub-form)
-(unless (memq (car-safe sub-form) test-duplicates-progn-forms)
+(unless (memq (car-safe form) test-duplicates-progn-forms)
   (push idx (alist-get
 		 sub-form
  (alist-get form-path test-duplicates-detector-forms
-- 
2.43.0

>From 77ec5f6e3834253a3ba872140f32b148b1135887 Mon Sep 17 00:00:00 2001
Message-ID: <77ec5f6e3834253a3ba872140f32b148b1135887.1706703212.git.yanta...@posteo.net>
In-Reply-To: <316c36d2b388a81e43fabf392a75af85532f56ba.1706703212.git.yanta...@posteo.net>
References: <316c36d2b388a81e43fabf392a75af85532f56ba.1706703212.git.yanta...@posteo.net>
From: Ihor Radchenko 
Date: Wed, 31 Jan 2024 13:12:49 +0100
Subject: [PATCH 2/3] ob-haskell/session-named-none-means-one-shot-sessions:
 Remove duplicate

*
testing/lisp/test-ob-haskell-ghci.el (ob-haskell/session-named-none-means-one-shot-sessions):
Remove duplicate from the test.
---
 testing/lisp/test-ob-haskell-ghci.el | 2 --
 1 file changed, 2 deletions(-)

diff --git a/testing/lisp/test-ob-haskell-ghci.el b/testing/lisp/test-ob-haskell-ghci.el
index cbd5f6f9a..990addcd4 100644
--- a/testing/lisp/test-ob-haskell-ghci.el
+++ b/testing/lisp/test-ob-haskell-ghci.el
@@ -117,8 +117,6 @@ (ert-deftest ob-haskell/sessions-must-not-share-variables ()
 (ert-deftest 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'."
-  (test-ob-haskell-ghci ":session none" "x=2" nil)
-  (should-not (equal 2 (test-ob-haskell-ghci ":session \"none\"" "x" nil)))
   (test-ob-haskell-ghci ":session none" "x=2" nil)
   (should-not (equal 2 (test-ob-haskell-ghci ":session \"none\"" "x" nil
 
-- 
2.43.0

>From 1f1c576cf5934e1c590faa7b0cad488aa8ffb9eb Mon Sep 17 00:00:00 2001
Message-ID: <1f1c576cf5934e1c590faa7b0cad488aa8ffb9eb.1706703212.git.yanta...@posteo.net>
In-Reply-To: <316c36d2b388a81e43fabf392a75af85532f56ba.1706703212.git.yanta...@posteo.net>
References: <316c36d2b388a81e43fabf392a75af85532f56ba.1706703212.git.yanta...@posteo.net>
From: Ihor Radchenko 
Date: Wed, 31 Jan 2024 13:13:11 +0100
Subject: [PATCH 3/3] 
 test-org-tests/test-duplicates-detector-testing-find-duplicates: Fix test

*
testing/lisp/test-duplicates-detector.el (test-org-tests/test-duplicates-detector-testing-find-duplicates):
Fix test condition.
---
 testing/lisp/test-duplicates-detector.el | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/testing/lisp/test-duplicates-detector.el b/testing/lisp/test-duplicates-detector.el
index b23fbce1e..25293f185 100644
--- a/testing/lisp/test-duplicates-detector.el
+++ b/testing/lisp/test-duplicates-detector.el
@@ -254,13 

Re: [PATCH] testing: Delete duplicate tests

[Ilya Chernyshov]

Ihor Radchenko  writes:

> What about the attached amendment?
> It should simplify things significantly.

Sorry, in my previous patch the test that checks the detector itself was not
even run in 'make test' because of incorrect test prefix.

Your patch does not work as you expect. Could you please revise it?

>From 6eb03414ac4eb8b64160b24dc7fcb805bf782310 Mon Sep 17 00:00:00 2001
From: Ihor Radchenko 
Date: Fri, 26 Jan 2024 14:21:55 +0100
Subject: [PATCH] test-duplicates-detector.el: Add correct prefix to the
 deftest, check if consp

*
testing/lisp/test-duplicates-detector.el (test-duplicates-detector--search-forms-recursively):

*
testing/lisp/test-duplicates-detector.el (test-org-tests/test-duplicates-detector-testing-find-duplicates):
Add correct prefix so that the test could be executed
---
 testing/lisp/test-duplicates-detector.el | 25 
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/testing/lisp/test-duplicates-detector.el b/testing/lisp/test-duplicates-detector.el
index d6f8aca5a..aed8034ee 100644
--- a/testing/lisp/test-duplicates-detector.el
+++ b/testing/lisp/test-duplicates-detector.el
@@ -231,22 +231,23 @@ FORM-PATH is list of the form:
 Write each form to `test-duplicates-detector-forms'"
   (let ((idx 0))
 (dolist (sub-form form)
-  (unless (memq (car-safe sub-form) test-duplicates-progn-forms)
-(push idx (alist-get
-		   sub-form
-   (alist-get form-path test-duplicates-detector-forms
-  nil nil #'equal)
-   nil nil #'equal-including-properties)))
-  (unless (memq (car-safe sub-form)
-		'(should-not should should-error))
-	(test-duplicates-detector--search-forms-recursively
- sub-form
- (append form-path (list (cons (car sub-form) idx)
+  (when (consp sub-form)
+(unless (memq (car-safe sub-form) test-duplicates-progn-forms)
+  (push idx (alist-get
+		 sub-form
+ (alist-get form-path test-duplicates-detector-forms
+nil nil #'equal)
+ nil nil #'equal-including-properties)))
+(unless (memq (car-safe sub-form)
+		  '(should-not should should-error))
+	  (test-duplicates-detector--search-forms-recursively
+   sub-form
+   (append form-path (list (cons (car-safe sub-form) idx))
   (cl-incf idx
 
  Testing the detector itself
 
-(ert-deftest test-duplicates-detector-testing-find-duplicates ()
+(ert-deftest test-org-tests/test-duplicates-detector-testing-find-duplicates ()
   "Test `test-duplicates-detector--find-duplicates'."
   (should
(equal
-- 
2.41.0

Re: [PATCH] testing: Delete duplicate tests

[Ihor Radchenko]

Ilya Chernyshov  writes:

> Ihor Radchenko  writes:
>
>> It has been a while since the last update in this thread.
>> Ilya, do you need any help with the patch?
>
> Hi, here is the updated patch.

Thanks!

What about the attached amendment?
It should simplify things significantly.

>From b14840aa81784547a6dd03aec510e23c898d97ec Mon Sep 17 00:00:00 2001
Message-ID: 
From: Ihor Radchenko 
Date: Fri, 26 Jan 2024 14:21:55 +0100
Subject: [PATCH] test-duplicates-detector.el: Do not search duplicates inside
 progn-like forms

*
testing/lisp/test-duplicates-detector.el (test-duplicates-progn-forms):
New variable holding a list of forms equivalent to `progn'.

*
testing/lisp/test-duplicates-detector.el (test-duplicates-detector--search-forms-recursively):
Refactor.  Skip duplicate checks inside immediate children of
`progn'-like constructs.

* testing/lisp/test-ob-R.el (test-ob-R/results-file):
* testing/lisp/test-ob-haskell-ghci.el (ob-haskell/session-named-none-means-one-shot-sessions):
* testing/lisp/test-ob-lob.el (test-ob-lob/call-with-header-arguments):
(test-ob-lob/caching-call-line):
(test-ob-lob/named-caching-call-line):
* testing/lisp/test-ob.el (test-ob/inline-src_blk-default-results-replace-line-1):
(test-ob/inline-src_blk-default-results-replace-line-2):
(test-ob/inline-src_blk-manual-results-replace):
(test-ob/inline-src_blk-results-silent):
(test-ob/just-one-results-block):
(test-ob/remove-inline-result):
(test-ob/goto-named-src-block):
* testing/lisp/test-ol.el (test-org-link/toggle-link-display):
* testing/lisp/test-org-agenda.el (test-org-agenda/property-timestamp):
(test-org-agenda/sticky-agenda-filter-preset):
* testing/lisp/test-org-element.el:
(test-org-element/cache-headline):
* testing/lisp/test-org-fold.el:
(test-org-fold/set-visibility-according-to-property):
* testing/lisp/test-org-list.el:
(test-org-list/list-navigation):
(test-org-list/move-item-down):
(test-org-list/move-item-up):
* testing/lisp/test-org-src.el:
(test-org-src/point-outside-block):
* testing/lisp/test-org-table.el:
(test-org-table/org-at-TBLFM-p):
(test-org-table/org-table-TBLFM-begin):
(test-org-table/org-table-TBLFM-begin-for-multiple-TBLFM-lines):
(test-org-table/org-table-TBLFM-begin-for-pultiple-TBLFM-lines-blocks):
* testing/lisp/test-org.el:
(test-org/goto-sibling):
(test-org/org-ctrl-c-ctrl-c):
(test-org/forward-element):
(test-org/backward-element):
(test-org/up-element): Remove unnecessary `org-test-ignore-duplicate'.
---
 testing/lisp/test-duplicates-detector.el |  34 ---
 testing/lisp/test-ob-R.el|  21 ++--
 testing/lisp/test-ob-haskell-ghci.el |   3 +-
 testing/lisp/test-ob-lob.el  |  87 -
 testing/lisp/test-ob.el  | 117 ++-
 testing/lisp/test-ol.el  |  35 ---
 testing/lisp/test-org-agenda.el  |   9 +-
 testing/lisp/test-org-element.el |  18 ++--
 testing/lisp/test-org-fold.el|  29 +++---
 testing/lisp/test-org-list.el|  31 +++---
 testing/lisp/test-org-src.el |   3 +-
 testing/lisp/test-org-table.el   |  93 +-
 testing/lisp/test-org.el |  27 ++
 13 files changed, 232 insertions(+), 275 deletions(-)

diff --git a/testing/lisp/test-duplicates-detector.el b/testing/lisp/test-duplicates-detector.el
index d39092a9e..d6f8aca5a 100644
--- a/testing/lisp/test-duplicates-detector.el
+++ b/testing/lisp/test-duplicates-detector.el
@@ -41,6 +41,14 @@ (require 'org-test "../testing/org-test")
 
  Variables
 
+(defvar test-duplicates-progn-forms
+  '( progn
+ org-test-with-temp-text
+ org-test-with-temp-text-in-file
+ org-test-ignore-duplicate)
+  "List of forms equivalent to `progn'.
+Immediate children inside these are not checked for duplicates.")
+
 (defvar test-duplicates-detector-file-path
   (expand-file-name "test-duplicates-detector.el"
 (expand-file-name "lisp" org-test-dir)))
@@ -221,22 +229,20 @@ (defun test-duplicates-detector--search-forms-recursively (form form-path)
 (symbol-1 . sexp-order-1) (symbol-2 . sexp-order-2))
 
 Write each form to `test-duplicates-detector-forms'"
-  (dotimes (iter (length form))
-(when (and
-	   (car-safe (nth iter form))
-	   (not
-	(eq (car-safe (nth iter form))
-		'org-test-ignore-duplicate)))
-  (push iter (alist-get
-		  (nth iter form)
-  (alist-get form-path test-duplicates-detector-forms
- nil nil #'equal)
-  nil nil #'equal-including-properties))
-  (unless (memq (car-safe (nth iter form))
+  (let ((idx 0))
+(dolist (sub-form form)
+  (unless (memq (car-safe sub-form) test-duplicates-progn-forms)
+(push idx (alist-get
+		   sub-form
+   (alist-get form-path test-duplicates-detector-forms
+  nil nil #'equal)
+   nil nil #'equal-including-properties)))
+  (unless (memq 

Re: [PATCH] testing: Delete duplicate tests

[Ilya Chernyshov]

Ihor Radchenko  writes:

> It has been a while since the last update in this thread.
> Ilya, do you need any help with the patch?

Hi, here is the updated patch.

>From 2385ba08a89f2966a6d71f92e8693e7def33e3fe Mon Sep 17 00:00:00 2001
From: Ilya Chernyshov 
Date: Mon, 22 Jan 2024 01:33:56 +0600
Subject: [PATCH] Add testing/lisp/test-duplicates-detector.el

* testing/lisp/test-duplicates-detector.el: Add test unit that checks for
duplicate ert-deftests and forms inside of them.

* testing/lisp/test-org.el (test-org/file-contents): Delete duplicate
`should-' form.

* testing/lisp/test-ob-lob.el (test-ob-lob/call-with-header-arguments,
test-ob-lob/do-not-eval-lob-lines-in-example-blocks-on-export,
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-ob-R.el (test-ob-R/results-file): Ignore
duplicate forms via `org-test-ignore-duplicate'.

* testing/lisp/test-ob-haskell-ghci.el (ob-haskell/session-named-none-means-one-shot-sessions): Ignore
duplicate forms via `org-test-ignore-duplicate'.

* testing/lisp/test-ob.el
(test-ob/inline-src_blk-default-results-replace-line-1,
test-ob/inline-src_blk-default-results-replace-line-2,
test-ob/inline-src_blk-manual-results-replace,
test-ob/inline-src_blk-results-silent, test-ob/just-one-results-block,
test-ob/remove-inline-result, test-ob/goto-named-src-block): Ignore
duplicate forms via `org-test-ignore-duplicate'.

* testing/lisp/test-org-agenda.el (test-org-agenda/property-timestamp,
test-org-agenda/sticky-agenda-filter-preset): Ignore
duplicate forms via `org-test-ignore-duplicate'

* testing/lisp/test-org-element.el (test-org-element/cache-headline):
Ignore duplicate forms via `org-test-ignore-duplicate'

* testing/lisp/test-org-fold.el (test-org-fold/set-visibility-according-to-property): Ignore duplicate
forms via `org-test-ignore-duplicate'

* testing/lisp/test-org-list.el (test-org-list/list-navigation,
test-org-list/move-item-down, test-org-list/move-item-up): Ignore duplicate
forms via `org-test-ignore-duplicate'

* testing/lisp/test-org-src.el (test-org-src/basic): Ignore duplicate
forms via `org-test-ignore-duplicate'

* testing/lisp/test-org-table.el (test-org-table/org-at-TBLFM-p,
test-org-table/org-table-TBLFM-begin,
test-org-table/org-table-TBLFM-begin-for-multiple-TBLFM-lines,
test-org-table/org-table-TBLFM-begin-for-pultiple-TBLFM-lines-blocks):
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,
test-org/org-ctrl-c-ctrl-c, test-org/forward-element): Ignore duplicate
forms via `org-test-ignore-duplicate'
---
 testing/lisp/test-duplicates-detector.el | 316 +++
 testing/lisp/test-ob-R.el|   5 +-
 testing/lisp/test-ob-haskell-ghci.el |   3 +-
 testing/lisp/test-ob-lob.el  |  87 ---
 testing/lisp/test-ob.el  | 113 
 testing/lisp/test-ol.el  |  35 +--
 testing/lisp/test-org-agenda.el  |   9 +-
 testing/lisp/test-org-element.el |  18 +-
 testing/lisp/test-org-fold.el|  29 ++-
 testing/lisp/test-org-list.el|  31 ++-
 testing/lisp/test-org-src.el |   3 +-
 testing/lisp/test-org-table.el   | 117 -
 testing/lisp/test-org.el |  38 ++-
 13 files changed, 568 insertions(+), 236 deletions(-)
 create mode 100644 testing/lisp/test-duplicates-detector.el

diff --git a/testing/lisp/test-duplicates-detector.el b/testing/lisp/test-duplicates-detector.el
new file mode 100644
index 0..d39092a9e
--- /dev/null
+++ b/testing/lisp/test-duplicates-detector.el
@@ -0,0 +1,316 @@
+;;; test-duplicates-detector.el --- Tests for finding duplicates in Org tests  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2023  Ilya Chernyshov
+;; Authors: Ilya Chernyshov 
+
+;; 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 .
+;;
+;;; Commentary:
+
+;; Unit tests that check for duplicate forms and tests in all Org test files.
+
+;; Forms are considered duplicate if they:
+
+;; 1. are `equal-including-properties',
+;; 2. have the same nesting 

Re: [PATCH] testing: Delete duplicate tests

[Ihor Radchenko]

Ilya Chernyshov  writes:

> 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.

It has been a while since the last update in this thread.
Ilya, do you need any help with the patch?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] testing: Delete duplicate tests

[Ilya Chernyshov]

Ilya Chernyshov  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?

Re: [PATCH] testing: Delete duplicate tests

[Ilya Chernyshov]

Ihor Radchenko  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.

>From 3b38450f7de8bd168d8795728454d9f4db720843 Mon Sep 17 00:00:00 2001
From: Ilya Chernyshov 
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 0..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 
+
+;; 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 .
+;;
+;;; 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 

Re: [PATCH] testing: Delete duplicate tests

[Ihor Radchenko]

Ihor Radchenko  writes:

> Ilya Chernyshov  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 .
Support Org development at ,
or support my work at 

Re: [PATCH] testing: Delete duplicate tests

[Ihor Radchenko]

Ilya Chernyshov  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 .
Support Org development at ,
or support my work at 

Re: [PATCH] testing: Delete duplicate tests

[Ilya Chernyshov]

Ihor Radchenko  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?

Re: [PATCH] testing: Delete duplicate tests

[Ihor Radchenko]

Ihor Radchenko  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 .
Support Org development at ,
or support my work at 

Re: [PATCH] testing: Delete duplicate tests

[Ihor Radchenko]

Max Nikulin  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 .
Support Org development at ,
or support my work at 

Re: [PATCH] testing: Delete duplicate tests

[Max Nikulin]

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.

Re: [PATCH] testing: Delete duplicate tests

[Ihor Radchenko]

Ilya Chernyshov  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| 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 .
Support Org development at ,
or support my work at 

[PATCH] testing: Delete duplicate tests

[Ilya Chernyshov]

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 ( 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:

>From 21ba128bd648c6737ed088abdd2a1824cfe01759 Mon Sep 17 00:00:00 2001
From: Ilya Chernyshov 
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-timest