Add the base of tests for lem-vi-mode #965

fukamachi · 2023-08-18T00:40:17Z

Usage

with-test-buffer creates a buffer object for testing
with-vi-buffer to set the initial vi state and bind the given buffer (or string) to be the current buffer
- Following functions are good to work with it:
  - cmd: Executes vi commands by key types in a string
    - ex) h, G, <Esc>, 3h, 5j^D, <C-x>o, <H-Shift-x>, <M-Space>.
  - buf=: Check if the current buffer text equals the given buffer's text, and the cursor position is right
  - text=: Check if the current buffer text equals the given buffer's text
  - pos=: Check if the current point is at the same position as the given point
  - state=: Check if the vi state is the given one.

Examples

Test definitions

[x] means a cursor that is on the character x. < and > means a region of vi's visual mode.

(deftest vi-forward-char
  (with-fake-interface ()
    (with-vi-buffer (#?"[a]bcdef\n")
      ;; `l`: vi-forward-char
      (cmd "l")
      (ok (buf= #?"a[b]cdef\n"))
      ;; `l` accepts a universal argument
      (cmd "3l")
      (ok (buf= #?"abcd[e]f\n"))
      ;; `l` doesn't exceed the end of the line
      (cmd "10l")
      (ok (buf= #?"abcde[f]\n"))
      (cmd "hv^")
      (ok (buf= #?"<[a]bcde>f\n"))
      (cmd "vlV")
      (ok (buf= #?"<a[b]cdef\n>")))))))

Test outputs

Testing System lem-vi-mode/tests

;; testing 'lem-vi-mode/tests/motion'
vi-forward-char
  [buf] "[a]bcdef\n"
    [cmd] l
    ✓ Expect the buffer to be "a[b]cdef\n" (actual: "a[b]cdef\n")
    [cmd] 3l
    ✓ Expect the buffer to be "abcd[e]f\n" (actual: "abcd[e]f\n")
    [cmd] 10l
    ✓ Expect the buffer to be "abcde[f]\n" (actual: "abcde[f]\n")
    [cmd] hv^
    ✓ Expect the buffer to be "<[a]bcde>f\n" (actual: "<[a]bcde>f\n")
    [cmd] vlV
    ✓ Expect the buffer to be "<a[b]cdef\n>" (actual: "<a[b]cdef\n>")

✓ 1 test completed

Summary:
  All 1 test passed.

cxxxr

The time spent reading this PR was very meaningful and educational.
I made some comments, but it is not MUST.

cxxxr · 2023-08-19T02:54:17Z

extensions/vi-mode/tests/utils.lisp

+(defmacro with-test-buffer ((var buffer-string
+                             &rest buffer-args
+                             &key name (temporary t temporary-specified-p)
+                             &allow-other-keys)
+                            &body body)
+  (declare (ignore temporary))
+  (unless temporary-specified-p
+    (setf (getf buffer-args :temporary) t))
+  (remove-from-plistf buffer-args :name)
+  (with-gensyms (point buffer-content position visual-regions)
+    `(with-fake-interface ()
+       (let* ((,var (make-buffer ,name ,@buffer-args))
+              (,point (buffer-point ,var)))
+         (multiple-value-bind (,buffer-content ,position ,visual-regions)
+             (parse-buffer-string ,buffer-string)
+           (insert-string ,point ,buffer-content)
+           (move-to-position ,point ,position)
+           (dolist (region ,visual-regions)
+             (destructuring-bind (from . to) region
+               (with-point ((start ,point)
+                            (end ,point))
+                 (move-to-position start from)
+                 (move-to-position end to)
+                 (push (lem:make-overlay start end 'lem:region)
+                       lem-vi-mode/visual::*visual-overlays*)))))
+         ,@body))))
+
+(defmacro with-current-buffer ((buffer) &body body)
+  (with-gensyms (window)
+    (once-only (buffer)
+      `(lem-base::with-current-buffers ()
+         (let ((lem-base::*current-buffer* ,buffer)
+               (,window (current-window)))
+           (lem-core::set-window-buffer ,buffer ,window)
+           (lem-core::set-window-view-point (copy-point (lem:buffer-point ,buffer))
+                                            ,window)
+           ,@body)))))
+
+(defmacro with-vi-state ((state) &body body)
+  `(let* ((lem-vi-mode/core::*current-state* (ensure-state (keyword-to-state ,state))))
+     (change-global-mode-keymap
+      'vi-mode
+      (lem-vi-mode/core::state-keymap lem-vi-mode/core::*current-state*))
+     ,@body))
+
+(defmacro with-vi-tests ((buffer &key (state :normal)) &body body)
+  `(with-current-buffer (,buffer)
+     (lem-core:change-buffer-mode ,buffer 'vi-mode)
+     (with-vi-state (,state)
+       (rove:testing (format nil "[buf] \"~A\""
+                             (text-backslashed
+                              (make-buffer-string (current-buffer))))
+         (locally
+             (declare #+sbcl (sb-ext:muffle-conditions sb-ext:code-deletion-note))
+           (labels ((cmd (keys)
+                      (check-type keys string)
+                      (rove:diag (format nil "[cmd] ~A" keys))
+                      (execute-key-sequence
+                       (parse-command-keys keys)))
+                    (pos= (expected-point)
+                      (point= (current-point) expected-point))
+                    (text= (expected-buffer-text)
+                      (string= (buffer-text (current-buffer))
+                               expected-buffer-text))
+                    (state= (expected-state)
+                      (eq (keyword-to-state expected-state)
+                          (current-state)))
+                    (buf= (expected-buffer-string)
+                      (check-type expected-buffer-string string)
+                      (multiple-value-bind (expected-buffer-text expected-position)
+                          (parse-buffer-string expected-buffer-string)
+                        (with-point ((p (current-point)))
+                          (move-to-position p expected-position)
+                          (and (text= expected-buffer-text)
+                               (pos= p))))))
+             ,@body))))))


https://google.github.io/styleguide/lispguide.xml?showone=Macros#Macros
Since huge macros are not maintainable, how about using call-with-style?

cxxxr · 2023-08-19T02:59:08Z

extensions/vi-mode/tests/motion.lisp

+(deftest vi-forward-char
+  (with-test-buffer (b #?"[a]bcdef\n")
+    (with-vi-tests (b)
+      (cmd "l")
+      (ok (buf= #?"a[b]cdef\n"))
+      (cmd "3l")
+      (ok (buf= #?"abcd[e]f\n"))
+      (cmd "10l")
+      (ok (buf= #?"abcde[f]\n")))))


This is really cool.

cxxxr · 2023-08-19T03:00:35Z

extensions/vi-mode/tests/utils.lisp

+  (:import-from :rove
+                :form-description
+                :diag
+                :testing)
+  (:import-from :lem-base
+                :*current-buffer*)
+  (:import-from :lem-fake-interface
+                :with-fake-interface)
+  (:import-from :lem-vi-mode
+                :vi-mode)
+  (:import-from :lem-vi-mode/core
+                :*current-state*
+                :state-keymap
+                :normal
+                :insert
+                :current-state
+                :ensure-state)
+  (:import-from :lem-vi-mode/visual
+                :visual-line
+                :visual-block
+                :apply-visual-range)
+  (:import-from :cl-ppcre
+                :scan
+                :regex-replace)
+  (:import-from :alexandria
+                :remove-from-plistf
+                :with-gensyms
+                :once-only
+                :appendf
+                :if-let)


It seems that there are symbols that have been imported but not used.
Can you check with M-x lisp-organize-import?

Why haven't I known this feature?
This must be one of the good reasons to recommend people to use Lem.

cxxxr · 2023-08-19T03:09:35Z

extensions/vi-mode/tests/utils.lisp

+    (setf (getf buffer-args :temporary) t))
+  (remove-from-plistf buffer-args :name)
+  (with-gensyms (point buffer-content position visual-regions)
+    `(with-fake-interface ()


I am a little concerned about the with-fake-interface being hidden inside this macro.
The person reading the test needs to dig into the macro to understand that this is using a fake-interface.
How about moving the use of with-fake-interface to the top level of testing?

fukamachi · 2023-08-19T05:02:36Z

@cxxxr
Thanks for your comments!
They're really helpful, and I applied all your advice :)

cxxxr · 2023-08-19T12:11:15Z

Thank you so much!

fukamachi added 5 commits August 18, 2023 09:34

Add tests for lem-vi-mode.

4078f23

Improve lem-vi-mode/tests/utils.

b8a64bd

Add support of visual mode to lem-vi-mode test utilities.

258b57d

Set the vi state in with-vi-tests locally.

0f58f37

Support special characters between <>.

f3b1a74

fukamachi force-pushed the vi-mode-tests branch from 061d146 to f3b1a74 Compare August 18, 2023 13:05

fukamachi added 2 commits August 18, 2023 22:10

Add the form-description method for state=.

058c691

Fix *current-state* variable.

1f65be2

fukamachi marked this pull request as ready for review August 18, 2023 13:18

fukamachi added the vi-mode label Aug 18, 2023

fukamachi added 2 commits August 19, 2023 05:49

Muffle code deletion note.

3043b68

Fix test utilities for vi-mode.

a41fcc5

cxxxr reviewed Aug 19, 2023

View reviewed changes

fukamachi added 3 commits August 19, 2023 13:58

Rewrite macros in vi-mode tests to be 'call-with-foo' style.

ab6c3a5

Stop using implicit with-fake-interface inside with-vi-tests.

674004d

M-x lisp-organize-imports.

6dfa792

fukamachi force-pushed the vi-mode-tests branch from b0512e4 to 6dfa792 Compare August 19, 2023 04:58

fukamachi requested a review from cxxxr August 19, 2023 05:04

fukamachi added 2 commits August 19, 2023 18:14

Include with-test-buffer into with-vi-tests.

13935c5

Revice test utilities for vi-mode. with-vi-tests -> with-vi-buffer.

83d1a79

fukamachi force-pushed the vi-mode-tests branch from 6bbbbd2 to 83d1a79 Compare August 19, 2023 11:07

Unnecessary let* -> let.

c1ec395

cxxxr merged commit 17c99fb into lem-project:main Aug 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the base of tests for lem-vi-mode #965

Add the base of tests for lem-vi-mode #965

fukamachi commented Aug 18, 2023 •

edited

Loading

cxxxr left a comment

cxxxr Aug 19, 2023

cxxxr Aug 19, 2023

cxxxr Aug 19, 2023

fukamachi Aug 19, 2023

cxxxr Aug 19, 2023

fukamachi commented Aug 19, 2023

cxxxr commented Aug 19, 2023

Add the base of tests for lem-vi-mode #965

Add the base of tests for lem-vi-mode #965

Conversation

fukamachi commented Aug 18, 2023 • edited Loading

Usage

Examples

Test definitions

Test outputs

cxxxr left a comment

Choose a reason for hiding this comment

cxxxr Aug 19, 2023

Choose a reason for hiding this comment

cxxxr Aug 19, 2023

Choose a reason for hiding this comment

cxxxr Aug 19, 2023

Choose a reason for hiding this comment

fukamachi Aug 19, 2023

Choose a reason for hiding this comment

cxxxr Aug 19, 2023

Choose a reason for hiding this comment

fukamachi commented Aug 19, 2023

cxxxr commented Aug 19, 2023

fukamachi commented Aug 18, 2023 •

edited

Loading