emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Maxim Nikulin <manikulin@gmail.com>
To: emacs-orgmode@gnu.org
Subject: Re: [PATCH] Re: Bug: Plain https links with brackets are not recognised [9.4.4 (release_9.4.4-625-g763c7a @ /home/yantar92/.emacs.d/straight/build/org/)]
Date: Fri, 19 Mar 2021 19:10:07 +0700	[thread overview]
Message-ID: <s324b0$74g$1@ciao.gmane.io> (raw)
In-Reply-To: <8735wvuvi3.fsf@localhost>

On 16/03/2021 19:35, Ihor Radchenko wrote:
> +;;; Link regexps
> +
> +(ert-deftest test-ol/plain-link-re ()
> +  "Test `org-link-plain-re'."
> +  (should
> +   (equal
> +    '("https" "//example.com/qwe()")
> +    (org-test-with-temp-text
> +        "(Some text in parenthesis followed by link with brackets <point>https://example.com/qwe())"
> +      (list (org-element-property :type (org-element-link-parser))
> +            (org-element-property :path (org-element-link-parser))))))
> +  (should
> +   (equal
> +    '("https" "//doi.org/10.1016/0160-791x(79)90023-x")
> +    (org-test-with-temp-text
> +        "<point>https://doi.org/10.1016/0160-791x(79)90023-x"
> +      (list (org-element-property :type (org-element-link-parser))
> +            (org-element-property :path (org-element-link-parser))))))

To be clear, I do not ask for any changes. It is great to have some 
tests even in the current form. I just have never tried ert before, so I 
have some questions.

Am I right that just one failure will be reported if a change in the 
regexp causes problems with several test inputs? I am afraid, it is 
rather inconvenient since this tests are rather for quality of 
heuristics than exact requirements. To find proper balance of accuracy 
and speed/regexp complexity, it is better to have all failures at once. 
I am looking up for a feature like EXPECT_EQ (delayed failure) in 
addition to strict ASSERT_EQ in googletest or at least like 
TestCase.subTest in puthon unittest. For this particular case even 
parametrized tests are enough (googletest, pytest).

I have tried implement something similar to illustrate my expectations. 
I think, for experienced lisp programmers the code looks ugly

(defmacro test-ol/deftest-parametrized
     (prefix func &rest cases)
   (declare (indent 2))
     (lambda (case)
       (pcase-let ((`(,id ,docstring ,expect ,arguments) case))
	(let ((test (intern (concat prefix "--" id)))
	      (exp (if (and expect (listp expect)) `'(,@expect) expect))
	      (args (if (listp arguments) arguments (list arguments))))
	  `(ert-deftest ,test ()
	     (should (equal ,exp ,(cons func args)))))))

(defun test-ol/match-link-plain (text)
   (and (string-match org-link-plain-re text)
        (mapcar (lambda (i) (match-string-no-properties i text))
	       ;; Currently there is a useless additional group
	       ;; and I think it should cause failure.
	    ;; (number-sequence 1 (- (/ (length (match-data)) 2) 1) 1)
	       '(1 2))))

(test-ol/deftest-parametrized "test-ol/org-link-plain-re"
   ("sheme-only" "No match without host or path"
    nil "Secure https: connection")
   ("scheme-slashes" "Questionable case with no host or path"
    ("file" "///") "Use file:/// for local files")
   ("simple-link" "Simple link"
    ("https" "//orgmode.org/") "Link https://orgmode.org/ to Org site")
   ("false-match" "Failure example: unexpected match"
    nil "Valid file:///bin/bash link")
   ("failed-match" "Failure example: host-path is too short to match"
    ("https" "a") "file:a")
   ("a-typo" "Failure example: slashes missed in expectation"
    ("http" "www.gnu.org") "http://www.gnu.org/"))

(ert t)

In my opinion, test report is acceptable


F test-ol/org-link-plain-re--a-typo
     Failure example: slashes missed in expectation
	'("http" "www.gnu.org")
	(test-ol/match-link-plain "http://www.gnu.org/")))
        ("http" "www.gnu.org")
        ("http" "//www.gnu.org/"))
       :value nil :explanation
       (list-elt 1
		(arrays-of-different-length 11 14 "www.gnu.org" "//www.gnu.org/" 
first-mismatch-at 0))))

F test-ol/org-link-plain-re--failed-match
     Failure example: host-path is too short to match
	'("https" "a")
	(test-ol/match-link-plain "file:a")))
        ("https" "a")
       :value nil :explanation
        ("https" "a")

F test-ol/org-link-plain-re--false-match
     Failure example: unexpected match
        (equal nil
	      (test-ol/match-link-plain "Valid file:///bin/bash link")))
       (equal nil
	     ("file" "///bin/bash"))
       :value nil :explanation
       (different-types nil
		       ("file" "///bin/bash"))))

I do not know if it is possible to implement "might" (in addition to 
"should") that using restart or some other technique will prevent 
immediate abandoning of the test but will mark whole test as failed at 
the end.

Actually I hope to get response that I am trying to reinvent a wheel and 
org or emacs has an established way to write tests feeding the same 
function with list of cases and to get all failures in a reasonably 
readable report.

  parent reply	other threads:[~2021-03-19 12:13 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-13  2:56 Bug: Plain https links with brackets are not recognised [9.4.4 (release_9.4.4-625-g763c7a @ /home/yantar92/.emacs.d/straight/build/org/)] Ihor Radchenko
2021-03-13  3:23 ` Kyle Meyer
2021-03-13  5:21   ` [PATCH] " Ihor Radchenko
2021-03-13  5:24     ` Ihor Radchenko
2021-03-15 11:54       ` Maxim Nikulin
2021-03-16 12:35         ` Ihor Radchenko
2021-03-17 14:59           ` Maxim Nikulin
2021-03-19 15:07             ` Ihor Radchenko
2021-03-24 12:31               ` Maxim Nikulin
2021-03-24 14:11                 ` Ihor Radchenko
2021-03-19 12:10           ` Maxim Nikulin [this message]
2021-03-19 15:40             ` Ihor Radchenko
2021-03-24 12:51 ` Nicolas Goaziou
2021-03-24 13:10   ` Ihor Radchenko
2021-03-24 14:13     ` Ihor Radchenko
2021-05-15  8:34       ` Bastien

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='s324b0$74g$1@ciao.gmane.io' \
    --to=manikulin@gmail.com \
    --cc=emacs-orgmode@gnu.org \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox


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