emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] org-test: Run all ol tests on make test
@ 2022-07-26 15:27 Max Nikulin
  2022-07-27  3:49 ` Ihor Radchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Max Nikulin @ 2022-07-26 15:27 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Hugo Heagren

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

Hi,

I figured out why "make test" is passing successfully while the same 
with BTEST_RE="test-ol/insert-link-insert-description" is failing for a 
proposed patch. Default selection pattern is ignoring most test-ol 
cases. See the attachment.

[-- Attachment #2: 0001-org-test-Run-all-ol-tests-on-make-test.patch --]
[-- Type: text/x-patch, Size: 1983 bytes --]

From 0cb30c77d888a38360f11657f7a391ab72fa312e Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Tue, 26 Jul 2022 22:11:40 +0700
Subject: [PATCH] org-test: Run all ol tests on make test

* mk/default.mk (BTEST_RE): Add -ol to the default pattern.
* testing/org-test.el (org-test-run-batch-tests): Update default regexp
for test selection in the docstring.

Only `test-ol/org-toggle-link-display' from test-ol.el was executed
since it is matched by "org" part of the pattern. The leading dash is
added to make tests a bit more specific. Reported in:
Ihor Radchenko to emacs-orgmode. Re [PATCH v7] ol.el: add
description format parameter to org-link-parameters.
Sun, 24 Jul 2022 21:15:31 +0800.
https://list.orgmode.org/87v8rmd53g.fsf@localhost/
---
 mk/default.mk       | 2 +-
 testing/org-test.el | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/mk/default.mk b/mk/default.mk
index 33e737da3..7371d3a99 100644
--- a/mk/default.mk
+++ b/mk/default.mk
@@ -72,7 +72,7 @@ REPRO_ARGS ?=
 req-ob-lang = --eval '(require '"'"'ob-$(ob-lang))'
 lst-ob-lang = ($(ob-lang) . t)
 req-extra   = --eval '(require '"'"'$(req))'
-BTEST_RE   ?= \\(org\\|ob\\)
+BTEST_RE   ?= \\(org\\|ob\\|-ol\\)
 BTEST_LOAD  = \
 	--eval '(add-to-list '"'"'load-path (concat default-directory "lisp"))' \
 	--eval '(add-to-list '"'"'load-path (concat default-directory "testing"))'
diff --git a/testing/org-test.el b/testing/org-test.el
index 0520e82f9..6defa9cc9 100644
--- a/testing/org-test.el
+++ b/testing/org-test.el
@@ -424,7 +424,7 @@ setting `pp-escape-newlines' to nil manually."
     "^\\([^.]\\|\\.\\([^.]\\|\\..\\)\\).*\\.org$")))
 
 (defun org-test-run-batch-tests (&optional org-test-selector)
-  "Run all tests matching an optional regex which defaults to \"\\(org\\|ob\\)\".
+  "Run all tests matching an optional regex which defaults to \"\\(org\\|ob\\|-ol\\)\".
 Load all test files first."
   (interactive)
   (let ((org-id-track-globally t)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] org-test: Run all ol tests on make test
  2022-07-26 15:27 [PATCH] org-test: Run all ol tests on make test Max Nikulin
@ 2022-07-27  3:49 ` Ihor Radchenko
  2022-07-27 15:43   ` Max Nikulin
  0 siblings, 1 reply; 6+ messages in thread
From: Ihor Radchenko @ 2022-07-27  3:49 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode, Hugo Heagren

Max Nikulin <manikulin@gmail.com> writes:

> Hi,
>
> I figured out why "make test" is passing successfully while the same 
> with BTEST_RE="test-ol/insert-link-insert-description" is failing for a 
> proposed patch. Default selection pattern is ignoring most test-ol 
> cases. See the attachment.

Thanks a lot for narrowing this down!

> --- a/testing/org-test.el
> +++ b/testing/org-test.el
> @@ -424,7 +424,7 @@ setting `pp-escape-newlines' to nil manually."
>      "^\\([^.]\\|\\.\\([^.]\\|\\..\\)\\).*\\.org$")))
>  
>  (defun org-test-run-batch-tests (&optional org-test-selector)
> -  "Run all tests matching an optional regex which defaults to \"\\(org\\|ob\\)\".
> +  "Run all tests matching an optional regex which defaults to \"\\(org\\|ob\\|-ol\\)\".
>  Load all test files first."
>    (interactive)
>    (let ((org-id-track-globally t)

You missed the

	(org-test-selector
	 (if org-test-selector org-test-selector "\\(org\\|ob\\)"))

inside org-test-run-batch-tests

Also, org-test-run-all-tests

Best,
Ihor


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] org-test: Run all ol tests on make test
  2022-07-27  3:49 ` Ihor Radchenko
@ 2022-07-27 15:43   ` Max Nikulin
  2022-07-28 14:31     ` Ihor Radchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Max Nikulin @ 2022-07-27 15:43 UTC (permalink / raw)
  To: emacs-orgmode

On 27/07/2022 10:49, Ihor Radchenko wrote:
> 
> You missed the
> 
> 	(org-test-selector
> 	 (if org-test-selector org-test-selector "\\(org\\|ob\\)"))

I have notices that cases in test-oc.el are named test-org-cite/... So I 
am considering renaming tests in test-ol.el in the same way from 
test-ol/... to test-org-link/... instead of modifying selectors. I have 
not noticed with approach to naming is recommended.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] org-test: Run all ol tests on make test
  2022-07-27 15:43   ` Max Nikulin
@ 2022-07-28 14:31     ` Ihor Radchenko
  2022-07-28 15:14       ` [PATCH v2] " Max Nikulin
  0 siblings, 1 reply; 6+ messages in thread
From: Ihor Radchenko @ 2022-07-28 14:31 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

Max Nikulin <manikulin@gmail.com> writes:

> On 27/07/2022 10:49, Ihor Radchenko wrote:
>> 
>> You missed the
>> 
>> 	(org-test-selector
>> 	 (if org-test-selector org-test-selector "\\(org\\|ob\\)"))
>
> I have notices that cases in test-oc.el are named test-org-cite/... So I 
> am considering renaming tests in test-ol.el in the same way from 
> test-ol/... to test-org-link/... instead of modifying selectors. I have 
> not noticed with approach to naming is recommended.

Either way is fine. Ideally, we should stick to org- prefix, as expected
from Elisp conventions. But we are already kind of breaking this
convention by having all the oc- ol-, etc files.

Best,
Ihor


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] org-test: Run all ol tests on make test
  2022-07-28 14:31     ` Ihor Radchenko
@ 2022-07-28 15:14       ` Max Nikulin
  2022-07-29 13:06         ` Ihor Radchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Max Nikulin @ 2022-07-28 15:14 UTC (permalink / raw)
  To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]

On 28/07/2022 21:31, Ihor Radchenko wrote:
> Max Nikulin writes:
> 
>> On 27/07/2022 10:49, Ihor Radchenko wrote:
>>>
>>> You missed the
>>>
>>> 	(org-test-selector
>>> 	 (if org-test-selector org-test-selector "\\(org\\|ob\\)"))
>>
>> I have notices that cases in test-oc.el are named test-org-cite/... So I
>> am considering renaming tests in test-ol.el in the same way from
>> test-ol/... to test-org-link/... instead of modifying selectors. I have
>> not noticed with approach to naming is recommended.
> 
> Either way is fine. Ideally, we should stick to org- prefix, as expected
> from Elisp conventions. But we are already kind of breaking this
> convention by having all the oc- ol-, etc files.

I have decided that I prefer to have org-test common prefix even though 
it causes discrepancy test-ol.el file vs. test-org-link/... cases.

[-- Attachment #2: 0001-test-ol-Rename-cases-to-match-the-default-selector.patch --]
[-- Type: text/x-patch, Size: 5754 bytes --]

From 725369323a1a3303820c2c92b116e8238b895bac Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Thu, 28 Jul 2022 22:08:57 +0700
Subject: [PATCH] test-ol: Rename cases to match the default selector

* testing/lisp/test-ol.el (test-ol/encode): Rename to
`test-org-link/encode'.
(test-ol/decode): Rename to `test-org-link/decode'.
(test-ol/encode-url-with-escaped-char): Rename to
`test-org-link/encode-url-with-escaped-char'.
(test-ol/org-toggle-link-display): Rename to
`test-org-link/toggle-link-display').
(test-ol/escape): Rename to `test-org-link/escape'.
(test-ol/unescape): Rename to `test-org-link/unescape'.
(test-ol/make-string): Rename to `test-org-link/make-string'.
(test-ol/store-link): Rename to `test-org-link/store-link'.
(test-ol/update-radio-target-regexp): Rename to
`test-org-link/update-radio-target-regexp'.
(test-ol/next-link): Rename to `test-org-link/next-link'.
(test-ol/previous-link): Rename to `test-org-link/previous-link'.
(test-ol/plain-link-re): Rename to `test-org-link/plain-link-re'.

Only `test-ol/org-toggle-link-display' was executed during
"make test" since it is the only name matched by the "org|ob" default
selector.  Reported in: Ihor Radchenko to emacs-orgmode.
Re [PATCH v7] ol.el: add description format parameter
to org-link-parameters.  Sun, 24 Jul 2022 21:15:31 +0800.
https://list.orgmode.org/87v8rmd53g.fsf@localhost/
---
 testing/lisp/test-ol.el | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/testing/lisp/test-ol.el b/testing/lisp/test-ol.el
index 429bb52ee..76881885d 100644
--- a/testing/lisp/test-ol.el
+++ b/testing/lisp/test-ol.el
@@ -22,7 +22,7 @@
 \f
 ;;; Decode and Encode Links
 
-(ert-deftest test-ol/encode ()
+(ert-deftest test-org-link/encode ()
   "Test `org-link-encode' specifications."
   ;; Regural test.
   (should (string= "Foo%3A%42ar" (org-link-encode "Foo:Bar" '(?\: ?\B))))
@@ -33,7 +33,7 @@
   ;; Encode a Unicode multibyte character.
   (should (string= "%E2%82%AC" (org-link-encode "€" '(?\€)))))
 
-(ert-deftest test-ol/decode ()
+(ert-deftest test-org-link/decode ()
   "Test `org-link-decode' specifications."
   ;; Decode an ASCII character.
   (should (string= "[" (org-link-decode "%5B")))
@@ -42,7 +42,7 @@
   ;; Decode a Unicode multibyte character.
   (should (string= "€" (org-link-decode "%E2%82%AC"))))
 
-(ert-deftest test-ol/encode-url-with-escaped-char ()
+(ert-deftest test-org-link/encode-url-with-escaped-char ()
   "Encode and decode a URL that includes an encoded char."
   (should
    (string= "http://some.host.com/form?&id=blah%2Bblah25"
@@ -50,7 +50,7 @@
 	     (org-link-encode "http://some.host.com/form?&id=blah%2Bblah25"
 			      '(?\s ?\[ ?\] ?%))))))
 
-(ert-deftest test-ol/org-toggle-link-display ()
+(ert-deftest test-org-link/toggle-link-display ()
   "Make sure that `org-toggle-link-display' is working.
 See https://github.com/yantar92/org/issues/4."
   (dolist (org-link-descriptive '(nil t))
@@ -77,7 +77,7 @@ See https://github.com/yantar92/org/issues/4."
 \f
 ;;; Escape and Unescape Links
 
-(ert-deftest test-ol/escape ()
+(ert-deftest test-org-link/escape ()
   "Test `org-link-escape' specifications."
   ;; No-op when there is no backslash or square bracket.
   (should (string= "foo" (org-link-escape "foo")))
@@ -101,7 +101,7 @@ See https://github.com/yantar92/org/issues/4."
   (should (string= "\\[\\[\\[foo\\]\\]\\]" (org-link-escape "[[[foo]]]")))
   (should (string= "\\[\\[foo\\]\\] bar" (org-link-escape "[[foo]] bar"))))
 
-(ert-deftest test-ol/unescape ()
+(ert-deftest test-org-link/unescape ()
   "Test `org-link-unescape' specifications."
   ;; No-op if there is no backslash.
   (should (string= "foo" (org-link-unescape "foo")))
@@ -122,7 +122,7 @@ See https://github.com/yantar92/org/issues/4."
   (should (string= "[[[foo]]]" (org-link-unescape "\\[\\[\\[foo\\]\\]\\]")))
   (should (string= "[[foo]] bar" (org-link-unescape "\\[\\[foo\\]\\] bar"))))
 
-(ert-deftest test-ol/make-string ()
+(ert-deftest test-org-link/make-string ()
   "Test `org-link-make-string' specifications."
   ;; Throw an error on empty URI.
   (should-error (org-link-make-string ""))
@@ -147,7 +147,7 @@ See https://github.com/yantar92/org/issues/4."
 \f
 ;;; Store links
 
-(ert-deftest test-ol/store-link ()
+(ert-deftest test-org-link/store-link ()
   "Test `org-store-link' specifications."
   ;; On a headline, link to that headline.  Use heading as the
   ;; description of the link.
@@ -380,7 +380,7 @@ See https://github.com/yantar92/org/issues/4."
 \f
 ;;; Radio Targets
 
-(ert-deftest test-ol/update-radio-target-regexp ()
+(ert-deftest test-org-link/update-radio-target-regexp ()
   "Test `org-update-radio-target-regexp' specifications."
   ;; Properly update cache with no previous radio target regexp.
   (should
@@ -414,7 +414,7 @@ See https://github.com/yantar92/org/issues/4."
 \f
 ;;; Navigation
 
-(ert-deftest test-ol/next-link ()
+(ert-deftest test-org-link/next-link ()
   "Test `org-next-link' specifications."
   ;; Move to any type of link.
   (should
@@ -465,7 +465,7 @@ See https://github.com/yantar92/org/issues/4."
 	    (org-next-link)
 	    (buffer-substring (point) (search-forward "]]" nil t))))))
 
-(ert-deftest test-ol/previous-link ()
+(ert-deftest test-org-link/previous-link ()
   "Test `org-previous-link' specifications."
   ;; Move to any type of link.
   (should
@@ -527,7 +527,7 @@ See https://github.com/yantar92/org/issues/4."
      (list (org-element-property :type (org-element-link-parser))
            (org-element-property :path (org-element-link-parser)))))
 
-(ert-deftest test-ol/plain-link-re ()
+(ert-deftest test-org-link/plain-link-re ()
   "Test `org-link-plain-re'."
   (should
    (equal
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] org-test: Run all ol tests on make test
  2022-07-28 15:14       ` [PATCH v2] " Max Nikulin
@ 2022-07-29 13:06         ` Ihor Radchenko
  0 siblings, 0 replies; 6+ messages in thread
From: Ihor Radchenko @ 2022-07-29 13:06 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

Max Nikulin <manikulin@gmail.com> writes:

> I have decided that I prefer to have org-test common prefix even though 
> it causes discrepancy test-ol.el file vs. test-org-link/... cases.
> From 725369323a1a3303820c2c92b116e8238b895bac Mon Sep 17 00:00:00 2001
> From: Max Nikulin <manikulin@gmail.com>
> Date: Thu, 28 Jul 2022 22:08:57 +0700
> Subject: [PATCH] test-ol: Rename cases to match the default selector

Thanks!
Applied onto main via 20274454c after removing a tangling ")" in the
commit message.

https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=20274454c6821b8194ba600d54c4d8ee996946c0

Best,
Ihor


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-07-29 13:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-26 15:27 [PATCH] org-test: Run all ol tests on make test Max Nikulin
2022-07-27  3:49 ` Ihor Radchenko
2022-07-27 15:43   ` Max Nikulin
2022-07-28 14:31     ` Ihor Radchenko
2022-07-28 15:14       ` [PATCH v2] " Max Nikulin
2022-07-29 13:06         ` 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).