GNU bug report logs -
#65734
29.1.50; kill-whole-line and visibility of Org subtrees
Previous Next
Reported by: Sebastian Miele <iota <at> whxvd.name>
Date: Mon, 4 Sep 2023 14:49:02 UTC
Severity: normal
Found in version 29.1.50
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
Full log
Message #83 received at 65734 <at> debbugs.gnu.org (full text, mbox):
> From: Eli Zaretskii <eliz <at> gnu.org>
> Date: Sun, 2023-09-10 19:57 +0300
>
>> From: Sebastian Miele <iota <at> whxvd.name>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>, Ihor Radchenko <yantar92 <at> posteo.net>
>> Date: Sun, 10 Sep 2023 18:31:20 +0200
>>
>> I removed emacs-orgmode <at> gnu.org from CC.
>>
>> > From: Sebastian Miele <iota <at> whxvd.name>
>> > Date: Wed, 2023-09-06 15:30 +0200
>> >
>> > I will write the tests. And I will probably come up with an updated
>> > version of the original patch. There is at least one cosmetic change.
>> > And something else that I want to have tried. May take some time.
>>
>> Please have a look at the following patch. For now it contains three
>> tests, two of them with :expected-result :failed. (They do not fail on
>> the bug-fixed version of `kill-whole-line'.)
>
> Yes, there should be more tests, ideally: there are situations where
> kill-whole-line signals an error,
Those tests are on the radar.
> and I don't think I see tests where some of the text is invisible (as
> the function uses forward-visible-line and end-of-visual-line).
That already is covered via (org-mode) and (org-fold-hide-sublevels 1)
in test `kill-whole-line-invisible'.
>> There probably will be more tests and further questions. But for now, I
>> would like to basically have a statement of whether the style of writing
>> the tests goes in an acceptable direction.
>
> Looks reasonable, but I'm not sure I understand what will the test
> show if one of the tests fails: will the information shown then tell
> enough to understand which of the sub-tests failed and why?
That almost certainly would not be immediately obvious in the current
state. I have next to no experience in working with testing frameworks.
But I assumed that regressions do not happen that often, and that it
would be good enough if the code of the test can be grasped quickly.
Then, in case of a regression, the test code can be temporarily
sprinkled with some printf-like debugging to find out the exact location
in the test.
However, enough of that printf-like debugging could also be hard-coded,
like in the following definition (see the line ending in the comment
"Provide some context"):
(ert-deftest kill-whole-line-read-only ()
;;:expected-result :failed
(cl-flet
((subtest (kill-whole-line-arg expected-kill-lines expected-buffer-lines)
(should `(subtest ,kill-whole-line-arg)) ; Provide some context
(ert-with-test-buffer-selected nil
(simple-tests--set-buffer-text-point-mark
(string-join '("-2" "-1" "A<POINT>B" "1" "2" "") "\n"))
(ert-simulate-command '(read-only-mode 1))
(should-error (ert-simulate-command
`(kill-whole-line ,kill-whole-line-arg))
:type 'buffer-read-only)
(should (equal (string-join expected-kill-lines "\n")
(car kill-ring)))
(should (equal (string-join expected-buffer-lines "\n")
(simple-tests--get-buffer-text-point-mark))))))
(subtest 0 '("AB") '("-2" "-1" "AB<POINT>" "1" "2" ""))
(subtest 1 '("AB" "") '("-2" "-1" "AB" "<POINT>1" "2" ""))
(subtest 2 '("AB" "1" "") '("-2" "-1" "AB" "1" "<POINT>2" ""))
(subtest 3 '("AB" "1" "2" "") '("-2" "-1" "AB" "1" "2" "<POINT>"))
(subtest 9 '("AB" "1" "2" "") '("-2" "-1" "AB" "1" "2" "<POINT>"))
(subtest -1 '("" "AB") '("-2" "-1<POINT>" "AB" "1" "2" ""))
(subtest -2 '("" "-1" "AB") '("-2<POINT>" "-1" "AB" "1" "2" ""))
(subtest -3 '("-2" "-1" "AB") '("<POINT>-2" "-1" "AB" "1" "2" ""))
(subtest -9 '("-2" "-1" "AB") '("<POINT>-2" "-1" "AB" "1" "2" ""))))
With the always succeeding
(should `(subtest ,kill-whole-line-arg)) ; Provide some context
at the beginning of ervery subtest, the context would be clear after
pressing l in a buffer showing the ERT test results (but never on the
console).
An alternative would be to use `message'. That would also provide the
context on the console. However, that also may be a bit noisy.
Another possibility would be to define every subtest as a top-level
test, by a macro like:
(defmacro simple-test--define-kill-whole-line-read-only-test (kill-whole-line-arg)
...)
But that feels a bit over the top and unflexible.
What to me feels like an ideal solution would be the concept of a
current context during an ERT test. Just something like a (defvar
ert-current-context) that always initially is dynamically let-bound to
nil during a test. That could be setq'ed at different locations in a
test to different arbitrary values (somewhat like ERT explanations).
When a should fails, and the ert-current-context is non-nil, ERT would
display the current context as the first information on the failed test.
WDYT?
For now I assume that providing context via
(should `(subtest ,kill-whole-line-arg)) ; Provide some context
is good enough.
This bug report was last modified 329 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.