unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
@ 2024-05-14 14:04 Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-14 16:52 ` Eli Zaretskii
  2024-05-15  2:36 ` Randy Taylor
  0 siblings, 2 replies; 23+ messages in thread
From: Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-14 14:04 UTC (permalink / raw)
  To: 70939


[-- Attachment #1.1: Type: text/plain, Size: 616 bytes --]

Hello folks,

This patch adds two new commands for go-ts-mode to run unit test cases.

The go-ts-mode-test-package command can run all the tests under the package
of
the current buffer. I've tested it to work with both Go module packages and
non-module packages. This command is bound to C-c C-p in the go-ts-mode-map.

The go-ts-mode-test-function-at-point command runs the current test
function. If
region is active then it runs all the test functions under the region. I'm
attaching a sample_test.go file as well for reviewers to test the patch.
This
command is bound to C-c C-t in the go-ts-mode-map.

-- 
Ankit

[-- Attachment #1.2: Type: text/html, Size: 688 bytes --]

[-- Attachment #2: sample_test.go --]
[-- Type: text/x-go, Size: 121 bytes --]

package sample_test

import "testing"

func TestIndividual(t *testing.T) {

}

func TestOverlapRegion(t *testing.T) {

}

[-- Attachment #3: 0001-Add-commands-to-run-unit-tests-in-go-ts-mode.patch --]
[-- Type: text/x-patch, Size: 4225 bytes --]

From 4988ef4962c8f3fde0ca0be7a1485488c7dca923 Mon Sep 17 00:00:00 2001
From: Ankit R Gadiya <git@argp.in>
Date: Tue, 14 May 2024 00:14:03 +0530
Subject: [PATCH] Add commands to run unit tests in go-ts-mode

---
 etc/NEWS                     | 10 ++++++
 lisp/progmodes/go-ts-mode.el | 63 +++++++++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 34052764f5f..b9df7fdcaf3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1236,6 +1236,16 @@ This command adds a docstring comment to the current defun.  If a
 comment already exists, point is only moved to the comment.  It is
 bound to 'C-c C-d' in 'go-ts-mode'.
 
+*** New unit test commands.
+Two new commands are now available to run unit tests.
+
+The 'go-ts-mode-test-function-at-point' command runs unit test at
+point. If a region is active, it runs all the unit tests under the
+region. It is bound to 'C-c C-t' in 'go-ts-mode'.
+
+The 'go-ts-mode-test-package' command runs all unit tests under the
+package of the current buffer. It is bound to 'C-c C-p' in 'go-ts-mode'.
+
 ** Man mode
 
 +++
diff --git a/lisp/progmodes/go-ts-mode.el b/lisp/progmodes/go-ts-mode.el
index aef224ab3fa..5540ddb61e6 100644
--- a/lisp/progmodes/go-ts-mode.el
+++ b/lisp/progmodes/go-ts-mode.el
@@ -237,7 +237,9 @@ go-ts-mode--font-lock-settings
 (defvar-keymap go-ts-mode-map
   :doc "Keymap used in Go mode, powered by tree-sitter"
   :parent prog-mode-map
-  "C-c C-d" #'go-ts-mode-docstring)
+  "C-c C-d" #'go-ts-mode-docstring
+  "C-c C-t" #'go-ts-mode-test-function-at-point
+  "C-c C-p" #'go-ts-mode-test-package)
 
 ;;;###autoload
 (define-derived-mode go-ts-mode prog-mode "Go"
@@ -370,6 +372,65 @@ go-ts-mode--comment-on-previous-line-p
      (<= (treesit-node-start node) point (treesit-node-end node))
      (string-equal "comment" (treesit-node-type node)))))
 
+(defun go-ts-mode--get-build-tags-flag ()
+  "Return compile flag for build tags.
+This function respects `go-build-tags' buffer-local variable for
+specifying build tags."
+  (if (local-variable-p 'go-build-tags)
+      (format "-tags %s" go-build-tags)
+    ""))
+
+(defun go-ts-mode--compile-test (regexp)
+  "Compiles the tests matching REGEXP.
+This function respects `go-build-tags' buffer-local variable for
+specifying build tags."
+  (compile (format "go test -v %s -run '%s'"
+                   (go-ts-mode--get-build-tags-flag)
+                   regexp)))
+
+(defun go-ts-mode--find-defun-at (start)
+  "Return the first defun node from START."
+  (let ((thing (or treesit-defun-type-regexp 'defun)))
+    (or (treesit-thing-at start thing)
+        (treesit-thing-next start thing))))
+
+(defun go-ts-mode--get-functions-in-range (start end)
+  "Return a list with names of all defuns in the range."
+  (let* ((node (go-ts-mode--find-defun-at start))
+	 (name (treesit-defun-name node))
+         (node-start (treesit-node-start node))
+         (node-end (treesit-node-end node)))
+    (if (or (not node)
+            (> start node-end)
+	    (< end node-start))
+	nil
+      (cons name (go-ts-mode--get-functions-in-range (treesit-node-end node) end)))))
+
+(defun go-ts-mode--get-test-regexp-at-point ()
+  "Return a regular expression for tests at point.
+If region is active, the regexp will include all the functions under the
+region."
+  (if (region-active-p)
+      (string-join (go-ts-mode--get-functions-in-range (region-beginning)
+                                                       (region-end))
+                   "|")
+    (treesit-defun-name (treesit-defun-at-point))))
+
+(defun go-ts-mode-test-function-at-point ()
+  "Run the unit test at point.
+If the point is anywhere in the test function, that function will be
+tested.  If the region is selected, all the functions under the region
+will be run."
+    (interactive)
+    (go-ts-mode--compile-test (go-ts-mode--get-test-regexp-at-point)))
+
+(defun go-ts-mode-test-package ()
+  "Run all the unit tests under current package."
+  (interactive)
+  (compile (format "go test -v %s -run %s"
+                   (go-ts-mode--get-build-tags-flag)
+                   default-directory)))
+
 ;; go.mod support.
 
 (defvar go-mod-ts-mode--syntax-table
-- 
2.39.2


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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-14 14:04 bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-14 16:52 ` Eli Zaretskii
  2024-05-14 17:24   ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-15  2:36 ` Randy Taylor
  1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2024-05-14 16:52 UTC (permalink / raw)
  To: Ankit Gadiya, Randy Taylor; +Cc: 70939

> Date: Tue, 14 May 2024 19:34:48 +0530
> From:  Ankit Gadiya via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Hello folks,
> 
> This patch adds two new commands for go-ts-mode to run unit test cases.
> 
> The go-ts-mode-test-package command can run all the tests under the package of
> the current buffer. I've tested it to work with both Go module packages and
> non-module packages. This command is bound to C-c C-p in the go-ts-mode-map.
> 
> The go-ts-mode-test-function-at-point command runs the current test function. If
> region is active then it runs all the test functions under the region. I'm
> attaching a sample_test.go file as well for reviewers to test the patch. This
> command is bound to C-c C-t in the go-ts-mode-map.

Thanks.

Randy, any comments about this?

Ankit, we'd need a copyright assignment from you to accept a large
contribution such as this one.  Would you like to start the assignment
paperwork at this time?  If yes, I will send you the form to fill and
the instructions to go with it.






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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-14 16:52 ` Eli Zaretskii
@ 2024-05-14 17:24   ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-14 17:59     ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-14 17:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Randy Taylor, 70939

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

>
> Ankit, we'd need a copyright assignment from you to accept a large
> contribution such as this one.  Would you like to start the assignment
> paperwork at this time?  If yes, I will send you the form to fill and
> the instructions to go with it.
>

Hello Eli, I've already sent the form to assign@gnu.org following the
instructions in the CONTRIBUTE file.

--
Ankit

[-- Attachment #2: Type: text/html, Size: 725 bytes --]

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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-14 17:24   ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-14 17:59     ` Eli Zaretskii
  0 siblings, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2024-05-14 17:59 UTC (permalink / raw)
  To: Ankit Gadiya; +Cc: dev, 70939

> From: Ankit Gadiya <ankit@argp.in>
> Date: Tue, 14 May 2024 22:54:22 +0530
> Cc: Randy Taylor <dev@rjt.dev>, 70939@debbugs.gnu.org
> 
>  Ankit, we'd need a copyright assignment from you to accept a large
>  contribution such as this one.  Would you like to start the assignment
>  paperwork at this time?  If yes, I will send you the form to fill and
>  the instructions to go with it.
> 
> Hello Eli, I've already sent the form to assign@gnu.org following the
> instructions in the CONTRIBUTE file.

Great, thanks.





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-14 14:04 bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-14 16:52 ` Eli Zaretskii
@ 2024-05-15  2:36 ` Randy Taylor
  2024-05-15  4:55   ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-15 11:21   ` Eli Zaretskii
  1 sibling, 2 replies; 23+ messages in thread
From: Randy Taylor @ 2024-05-15  2:36 UTC (permalink / raw)
  To: Ankit Gadiya; +Cc: Eli Zaretskii, 70939

On Tuesday, May 14th, 2024 at 10:04, Ankit Gadiya via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> wrote:
> Hello folks,
> 
> This patch adds two new commands for go-ts-mode to run unit test cases.
> 
> The go-ts-mode-test-package command can run all the tests under the package of
> the current buffer. I've tested it to work with both Go module packages and
> non-module packages. This command is bound to C-c C-p in the go-ts-mode-map.
> 
> The go-ts-mode-test-function-at-point command runs the current test function. If
> region is active then it runs all the test functions under the region. I'm
> attaching a sample_test.go file as well for reviewers to test the patch. This
> command is bound to C-c C-t in the go-ts-mode-map.
> 
> --
> Ankit

Thanks for working on this, it would certainly be a nice addition.

This is going to need a commit log entry. See the "Commit messages" section
in the CONTRIBUTING file.

I'm undecided on the keybinds. I think I would prefer something like:
C-c t p
or
C-c C-t p
so we can keep test-related things together.

I haven't tried this out yet, but here are some comments (mostly nits)
after a quick look:

In NEWS, sentences should be separated by 2 spaces.

+The 'go-ts-mode-test-function-at-point' command runs unit test at
                                                     ^the
+point.

+This function respects `go-build-tags' buffer-local variable
                       ^the

+  "Compiles the tests matching REGEXP.
    ^Compile

+If the point is anywhere in the test function, that function will be
+tested.
^ run (keeps it consistent with the next sentence)

+  "Run all the unit tests under current package."
                                ^the





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-15  2:36 ` Randy Taylor
@ 2024-05-15  4:55   ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-16  2:32     ` Randy Taylor
  2024-05-15 11:21   ` Eli Zaretskii
  1 sibling, 1 reply; 23+ messages in thread
From: Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-15  4:55 UTC (permalink / raw)
  To: Randy Taylor; +Cc: Eli Zaretskii, 70939

> This is going to need a commit log entry. See the "Commit messages" section
> in the CONTRIBUTING file.

I missed it earlier, will add it.

> I'm undecided on the keybinds. I think I would prefer something like:
> C-c t p
> or
> C-c C-t p
> so we can keep test-related things together.

Keeping it under C-c C-t makes sense to me. How about this:

C-c C-t t - go-ts-mode-test-function-at-point
C-c C-t p - go-ts-mode-test-package

> I haven't tried this out yet, but here are some comments (mostly nits)
> after a quick look:
>
> In NEWS, sentences should be separated by 2 spaces.
>
> +The 'go-ts-mode-test-function-at-point' command runs unit test at
>                                                      ^the
> +point.
>
> +This function respects `go-build-tags' buffer-local variable
>                        ^the
>
> +  "Compiles the tests matching REGEXP.
>     ^Compile
>
> +If the point is anywhere in the test function, that function will be
> +tested.
> ^ run (keeps it consistent with the next sentence)
>
> +  "Run all the unit tests under current package."
>                                 ^the

Thanks, I'll update this along with keybinding changes.

-- 
Ankit





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-15  2:36 ` Randy Taylor
  2024-05-15  4:55   ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-15 11:21   ` Eli Zaretskii
  2024-05-16  1:24     ` Randy Taylor
  1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2024-05-15 11:21 UTC (permalink / raw)
  To: Randy Taylor; +Cc: ankit, 70939

> Date: Wed, 15 May 2024 02:36:15 +0000
> From: Randy Taylor <dev@rjt.dev>
> Cc: 70939@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> 
> I'm undecided on the keybinds. I think I would prefer something like:
> C-c t p
> or
> C-c C-t p
> so we can keep test-related things together.

In general, key bindings that begin with "C-c" and a letter are
reserved for users.  So the second alternative is preferred.





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-15 11:21   ` Eli Zaretskii
@ 2024-05-16  1:24     ` Randy Taylor
  0 siblings, 0 replies; 23+ messages in thread
From: Randy Taylor @ 2024-05-16  1:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: ankit, 70939

On Wednesday, May 15th, 2024 at 07:21, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> 
> > Date: Wed, 15 May 2024 02:36:15 +0000
> 
> > From: Randy Taylor dev@rjt.dev
> > Cc: 70939@debbugs.gnu.org, Eli Zaretskii eliz@gnu.org
> > 
> > I'm undecided on the keybinds. I think I would prefer something like:
> > C-c t p
> > or
> > C-c C-t p
> > so we can keep test-related things together.
> 
> 
> In general, key bindings that begin with "C-c" and a letter are
> reserved for users. So the second alternative is preferred.

Noted, thanks.





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-15  4:55   ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-16  2:32     ` Randy Taylor
  2024-05-16  8:27       ` Eli Zaretskii
  2024-05-17  2:27       ` Randy Taylor
  0 siblings, 2 replies; 23+ messages in thread
From: Randy Taylor @ 2024-05-16  2:32 UTC (permalink / raw)
  To: Ankit Gadiya; +Cc: Eli Zaretskii, 70939

On Wednesday, May 15th, 2024 at 00:55, Ankit Gadiya <ankit@argp.in> wrote:
> 
> 
> > This is going to need a commit log entry. See the "Commit messages" section
> 
> > in the CONTRIBUTING file.
> 
> 
> I missed it earlier, will add it.
> 
> > I'm undecided on the keybinds. I think I would prefer something like:
> > C-c t p
> > or
> > C-c C-t p
> > so we can keep test-related things together.
> 
> 
> Keeping it under C-c C-t makes sense to me. How about this:
> 
> C-c C-t t - go-ts-mode-test-function-at-point
> C-c C-t p - go-ts-mode-test-package

Sounds good.

I'm wondering If C-c C-t t should be f for "function" but let's leave it as t for now. I do like the t for test which is nice and simple. Decisions, decisions...

> 
> > I haven't tried this out yet, but here are some comments (mostly nits)
> > after a quick look:
> > 
> > In NEWS, sentences should be separated by 2 spaces.
> > 
> > +The 'go-ts-mode-test-function-at-point' command runs unit test at
> > ^the
> > +point.
> > 
> > +This function respects `go-build-tags' buffer-local variable
> > ^the
> > 
> > + "Compiles the tests matching REGEXP.
> > ^Compile
> > 
> > +If the point is anywhere in the test function, that function will be
> > +tested.
> > ^ run (keeps it consistent with the next sentence)
> > 
> > + "Run all the unit tests under current package."
> > ^the
> 
> 
> Thanks, I'll update this along with keybinding changes.

Great. I'll try to give this a try tomorrow.

Eli, is there a convention regarding local variables?
In the patch we have:
+  (if (local-variable-p 'go-build-tags)
+      (format "-tags %s" go-build-tags)
+    ""))

Should that local variable be prefixed with go-ts-mode, or is it fine as is?

> 
> --
> Ankit





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-16  2:32     ` Randy Taylor
@ 2024-05-16  8:27       ` Eli Zaretskii
  2024-05-16 15:03         ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-17  2:27       ` Randy Taylor
  1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2024-05-16  8:27 UTC (permalink / raw)
  To: Randy Taylor; +Cc: ankit, 70939

> Date: Thu, 16 May 2024 02:32:42 +0000
> From: Randy Taylor <dev@rjt.dev>
> Cc: 70939@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> 
> Eli, is there a convention regarding local variables?
> In the patch we have:
> +  (if (local-variable-p 'go-build-tags)
> +      (format "-tags %s" go-build-tags)
> +    ""))
> 
> Should that local variable be prefixed with go-ts-mode, or is it fine as is?

If the variable is specific to go-ts-mode, it should indeed be
prefixed with "go-ts-".  But I don't see this variable in
go-ts-mode.el nor in the patch (did I miss something?), and I don't
understand why the above condition insists on using only a
buffer-local value of the variable in the first place?  What's wrong
with having a global value for the variable?





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-16  8:27       ` Eli Zaretskii
@ 2024-05-16 15:03         ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-16 16:01           ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-16 15:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Randy Taylor, 70939

> > Eli, is there a convention regarding local variables?
> > In the patch we have:
> > +  (if (local-variable-p 'go-build-tags)
> > +      (format "-tags %s" go-build-tags)
> > +    ""))
> >
> > Should that local variable be prefixed with go-ts-mode, or is it fine as is?
>
> If the variable is specific to go-ts-mode, it should indeed be
> prefixed with "go-ts-".  But I don't see this variable in
> go-ts-mode.el nor in the patch (did I miss something?), and I don't
> understand why the above condition insists on using only a
> buffer-local value of the variable in the first place?  What's wrong
> with having a global value for the variable?

I'm not yet familiar with Elisp-idioms but I'll explain my idea and why I used
the variable this way.

The build tags can be unique to the specific project and sometimes even per
go package. So I thought to make it buffer specific rather than a global
variable defined in the init.el. I'm using these changes on my machine and I'm
defining the build tags in the dir-locals file. This way the variable is always
available for the mode commands to access.

But now I'm thinking maybe it makes sense to add it as defvar-local under the
go-ts-mode? Please guide me with what will be the correct way to implement it.

-- 
Ankit





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-16 15:03         ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-16 16:01           ` Eli Zaretskii
  2024-05-18  9:54             ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2024-05-16 16:01 UTC (permalink / raw)
  To: Ankit Gadiya; +Cc: dev, 70939

> From: Ankit Gadiya <ankit@argp.in>
> Date: Thu, 16 May 2024 20:33:31 +0530
> Cc: Randy Taylor <dev@rjt.dev>, 70939@debbugs.gnu.org
> 
> > If the variable is specific to go-ts-mode, it should indeed be
> > prefixed with "go-ts-".  But I don't see this variable in
> > go-ts-mode.el nor in the patch (did I miss something?), and I don't
> > understand why the above condition insists on using only a
> > buffer-local value of the variable in the first place?  What's wrong
> > with having a global value for the variable?
> 
> I'm not yet familiar with Elisp-idioms but I'll explain my idea and why I used
> the variable this way.
> 
> The build tags can be unique to the specific project and sometimes even per
> go package. So I thought to make it buffer specific rather than a global
> variable defined in the init.el. I'm using these changes on my machine and I'm
> defining the build tags in the dir-locals file. This way the variable is always
> available for the mode commands to access.

It's okay to support buffer-local values, but using the value of the
variable will do that automatically; you don't have to verify it has a
local binding.  What I don't understand is why does your code _reject_
global bindings.  If some user wants to set up a global value for some
reason, or have a default global value and special local values in
some cases, why should we prevent that?

> But now I'm thinking maybe it makes sense to add it as defvar-local under the
> go-ts-mode? Please guide me with what will be the correct way to implement it.

Yes, defvar-local could be what we want.  But I first would like to
understand what is special about this variable that it should be
buffer-local.





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-16  2:32     ` Randy Taylor
  2024-05-16  8:27       ` Eli Zaretskii
@ 2024-05-17  2:27       ` Randy Taylor
  2024-05-18  8:55         ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 23+ messages in thread
From: Randy Taylor @ 2024-05-17  2:27 UTC (permalink / raw)
  To: Ankit Gadiya; +Cc: Eli Zaretskii, 70939

On Wednesday, May 15th, 2024 at 22:32, Randy Taylor <dev@rjt.dev> wrote:
> 
> Great. I'll try to give this a try tomorrow.

Hi Ankit,

I've played around a little bit and noticed an issue when using go-ts-mode-test-function-at-point:
- When there is no region selected, the test could run more than the function at point
  if any function names are the same but differ in suffixes (e.g. TestQuack and TestQuackMoo
  would both run if invoked inside just TestQuack). The same applies to when a region is
  selected. Everything within the region is run, and potentially stuff outside of it too.
  We need to make the regex strictly the functions we want to run with '^TestQuack$'.





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-17  2:27       ` Randy Taylor
@ 2024-05-18  8:55         ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 23+ messages in thread
From: Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-18  8:55 UTC (permalink / raw)
  To: Randy Taylor; +Cc: Eli Zaretskii, 70939

> I've played around a little bit and noticed an issue when using go-ts-mode-test-function-at-point:
> - When there is no region selected, the test could run more than the function at point
>   if any function names are the same but differ in suffixes (e.g. TestQuack and TestQuackMoo
>   would both run if invoked inside just TestQuack). The same applies to when a region is
>   selected. Everything within the region is run, and potentially stuff outside of it too.
>   We need to make the regex strictly the functions we want to run with '^TestQuack$'.

Thanks for trying out the patch. Yes, I now see how it can capture functions
with the same prefix. I will make the regexp stricter and update the
sample_test.go as well with common prefix examples.

--
Ankit





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-16 16:01           ` Eli Zaretskii
@ 2024-05-18  9:54             ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-25  2:35               ` Randy Taylor
  0 siblings, 1 reply; 23+ messages in thread
From: Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-18  9:54 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: dev, 70939

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

> It's okay to support buffer-local values, but using the value of the
> variable will do that automatically; you don't have to verify it has a
> local binding.  What I don't understand is why does your code _reject_
> global bindings.  If some user wants to set up a global value for some
> reason, or have a default global value and special local values in
> some cases, why should we prevent that?

You are absolutely right, now I realize that local-variable-p is overly
conservative. Since I didn't define the variable in the first place I had to do
the check and even then boundp would have been better.

> > But now I'm thinking maybe it makes sense to add it as defvar-local under the
> > go-ts-mode? Please guide me with what will be the correct way to implement it.
>
> Yes, defvar-local could be what we want.  But I first would like to
> understand what is special about this variable that it should be
> buffer-local.

With the improved understanding, I believe defcustom would be better for the
variable. Then I don't even need the the boundp check since the variable is
always present and it can be replaced by just the non-nil check instead.

As an improvement, I'm also changing the variable-type to a list of strings
instead of a single comma separated string.

I'm submitting an updated Patch with the following changes:
* Add commit log
* Fix the text formatting.
* Update the keybindings to use the C-c C-t prefix.
* Improve regexp matching to be more strict.
* Define =go-ts-mode-build-tags= variable in the module and use it in the test
  functions.

-- 
Ankit

[-- Attachment #2: 0001-Add-commands-to-run-unit-tests-in-go-ts-mode.patch --]
[-- Type: text/x-patch, Size: 5735 bytes --]

From cc6b3267e350861271e366bb3a6c2891cfa8557e Mon Sep 17 00:00:00 2001
From: Ankit R Gadiya <git@argp.in>
Date: Tue, 14 May 2024 00:14:03 +0530
Subject: [PATCH] Add commands to run unit tests in go-ts-mode

Two new commands are added in the go-ts-mode to run unit tests.
The go-ts-mode-test-function-at-point command runs the current
test function at point or all the functions in the active region
when region is active.  It is bound to "C-c C-t t". The
go-ts-mode-test-package command runs all tests in the current
package.  It is bound to "C-c C-t p".

* lisp/progmodes/go-ts-mode.el (go-ts-mode-build-tags): New variable.
(go-ts-mode-map): New map variable.
(go-ts-mode--get-build-tags-flag): New function.
(go-ts-mode--compile-test): New function.
(go-ts-mode--find-defun-at): New function.
(go-ts-mode--get-function-regexp): New function.
(go-ts-mode--get-functions-in-range): New function.
(go-ts-mode--get-test-regexp-at-point): New function.
(go-ts-mode-test-function-at-point): New function.
(go-ts-mode-test-package): New function.
* etc/NEWS: Mention the change.

(Bug#70939)
---
 etc/NEWS                     | 13 +++++++
 lisp/progmodes/go-ts-mode.el | 73 +++++++++++++++++++++++++++++++++++-
 2 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 77b2749fe43..c978c793503 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1236,6 +1236,19 @@ This command adds a docstring comment to the current defun.  If a
 comment already exists, point is only moved to the comment.  It is
 bound to 'C-c C-d' in 'go-ts-mode'.
 
+*** New unit test commands.
+Two new commands are now available to run unit tests.
+
+The 'go-ts-mode-test-function-at-point' command runs the unit test at
+point.  If a region is active, it runs all the unit tests under the
+region.  It is bound to 'C-c C-t' in 'go-ts-mode'.
+
+The 'go-ts-mode-test-package' command runs all unit tests under the
+package of the current buffer.  It is bound to 'C-c C-p' in 'go-ts-mode'.
+
+The 'go-ts-mode-build-tags' variable is available to set a list of build
+tags for the test commands.
+
 ** Man mode
 
 +++
diff --git a/lisp/progmodes/go-ts-mode.el b/lisp/progmodes/go-ts-mode.el
index aef224ab3fa..f6d872b5210 100644
--- a/lisp/progmodes/go-ts-mode.el
+++ b/lisp/progmodes/go-ts-mode.el
@@ -46,6 +46,12 @@ go-ts-mode-indent-offset
   :safe 'integerp
   :group 'go)
 
+(defcustom go-ts-mode-build-tags nil
+  "List of go build tags for the test commands."
+  :version "30.1"
+  :type '(repeat string)
+  :group 'go)
+
 (defvar go-ts-mode--syntax-table
   (let ((table (make-syntax-table)))
     (modify-syntax-entry ?+   "."      table)
@@ -237,7 +243,9 @@ go-ts-mode--font-lock-settings
 (defvar-keymap go-ts-mode-map
   :doc "Keymap used in Go mode, powered by tree-sitter"
   :parent prog-mode-map
-  "C-c C-d" #'go-ts-mode-docstring)
+  "C-c C-d" #'go-ts-mode-docstring
+  "C-c C-t t" #'go-ts-mode-test-function-at-point
+  "C-c C-t p" #'go-ts-mode-test-package)
 
 ;;;###autoload
 (define-derived-mode go-ts-mode prog-mode "Go"
@@ -370,6 +378,69 @@ go-ts-mode--comment-on-previous-line-p
      (<= (treesit-node-start node) point (treesit-node-end node))
      (string-equal "comment" (treesit-node-type node)))))
 
+(defun go-ts-mode--get-build-tags-flag ()
+  "Return compile flag for build tags.
+This function respects the `go-ts-mode-build-tags' variable for
+specifying build tags."
+  (if go-ts-mode-build-tags
+      (format "-tags %s" (string-join go-ts-mode-build-tags ","))
+    ""))
+
+(defun go-ts-mode--compile-test (regexp)
+  "Compile the tests matching REGEXP.
+This function respects `go-ts-mode-build-tags' variable for specifying
+build tags."
+  (compile (format "go test -v %s -run '%s'"
+                   (go-ts-mode--get-build-tags-flag)
+                   regexp)))
+
+(defun go-ts-mode--find-defun-at (start)
+  "Return the first defun node from START."
+  (let ((thing (or treesit-defun-type-regexp 'defun)))
+    (or (treesit-thing-at start thing)
+        (treesit-thing-next start thing))))
+
+(defun go-ts-mode--get-function-regexp (name)
+  (format "^%s$" name))
+
+(defun go-ts-mode--get-functions-in-range (start end)
+  "Return a list with names of all defuns in the range."
+  (let* ((node (go-ts-mode--find-defun-at start))
+	 (name (treesit-defun-name node))
+         (node-start (treesit-node-start node))
+         (node-end (treesit-node-end node)))
+    (if (or (not node)
+            (> start node-end)
+	    (< end node-start))
+	nil
+      (cons (go-ts-mode--get-function-regexp name)
+            (go-ts-mode--get-functions-in-range (treesit-node-end node) end)))))
+
+(defun go-ts-mode--get-test-regexp-at-point ()
+  "Return a regular expression for tests at point.
+If region is active, the regexp will include all the functions under the
+region."
+  (if (region-active-p)
+      (string-join (go-ts-mode--get-functions-in-range (region-beginning)
+                                                       (region-end))
+                   "|")
+    (go-ts-mode--get-function-regexp (treesit-defun-name (treesit-defun-at-point)))))
+
+(defun go-ts-mode-test-function-at-point ()
+  "Run the unit test at point.
+If the point is anywhere in the test function, that function will be
+run.  If the region is selected, all the functions under the region will
+be run."
+    (interactive)
+    (go-ts-mode--compile-test (go-ts-mode--get-test-regexp-at-point)))
+
+(defun go-ts-mode-test-package ()
+  "Run all the unit tests under the current package."
+  (interactive)
+  (compile (format "go test -v %s -run %s"
+                   (go-ts-mode--get-build-tags-flag)
+                   default-directory)))
+
 ;; go.mod support.
 
 (defvar go-mod-ts-mode--syntax-table
-- 
2.39.2


[-- Attachment #3: sample_test.go --]
[-- Type: text/x-go, Size: 167 bytes --]

package sample_test

import "testing"

func TestIndividual(t *testing.T) {

}

func TestIndividualFooBar(t *testing.T) {

}

func TestOverlapRegion(t *testing.T) {

}

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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-18  9:54             ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-25  2:35               ` Randy Taylor
  2024-05-28  2:30                 ` Randy Taylor
  0 siblings, 1 reply; 23+ messages in thread
From: Randy Taylor @ 2024-05-25  2:35 UTC (permalink / raw)
  To: Ankit Gadiya; +Cc: Eli Zaretskii, 70939

On Saturday, May 18th, 2024 at 05:54, Ankit Gadiya via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> wrote:
> 
> [...]
> 
> I'm submitting an updated Patch with the following changes:
> * Add commit log
> * Fix the text formatting.
> * Update the keybindings to use the C-c C-t prefix.
> * Improve regexp matching to be more strict.
> * Define =go-ts-mode-build-tags= variable in the module and use it in the test
> functions.
> 
> --
> Ankit

Thanks.

Sorry for the delay in reviewing, I've been having internet troubles since last Friday. I'll take a look at this next week.

A few quick things I noticed on a glance:

+  (let* ((node (go-ts-mode--find-defun-at start))
+	 (name (treesit-defun-name node))

Indentation is off on the name line - looks like a TAB was used? Should only be spaces everywhere. Double check the rest is OK.

+region.  It is bound to 'C-c C-t' in 'go-ts-mode'.
                         ^ C-c C-t t

+package of the current buffer.  It is bound to 'C-c C-p' in 'go-ts-mode'.
                                                ^ C-c C-t p

+  "List of go build tags for the test commands."
            ^ Go

+  "Return a list with names of all defuns in the range."
We should probably say what the range actually is (START to END) - not sure if we have a convention for that wording already.





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-25  2:35               ` Randy Taylor
@ 2024-05-28  2:30                 ` Randy Taylor
  2024-05-28 19:58                   ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 23+ messages in thread
From: Randy Taylor @ 2024-05-28  2:30 UTC (permalink / raw)
  To: Ankit Gadiya; +Cc: Eli Zaretskii, 70939

On Friday, May 24th, 2024 at 22:35, Randy Taylor <dev@rjt.dev> wrote:
> 
> On Saturday, May 18th, 2024 at 05:54, Ankit Gadiya via "Bug reports for GNU Emacs, the Swiss army knife of text editors" bug-gnu-emacs@gnu.org wrote:
> 
> > [...]
> > 
> > I'm submitting an updated Patch with the following changes:
> > * Add commit log
> > * Fix the text formatting.
> > * Update the keybindings to use the C-c C-t prefix.
> > * Improve regexp matching to be more strict.
> > * Define =go-ts-mode-build-tags= variable in the module and use it in the test
> > functions.
> > 
> > --
> > Ankit
> 
> 
> Thanks.
> 
> Sorry for the delay in reviewing, I've been having internet troubles since last Friday. I'll take a look at this next week.
> 
> A few quick things I noticed on a glance:
> 
> + (let* ((node (go-ts-mode--find-defun-at start))
> + (name (treesit-defun-name node))
> 
> Indentation is off on the name line - looks like a TAB was used? Should only be spaces everywhere. Double check the rest is OK.
> 
> +region. It is bound to 'C-c C-t' in 'go-ts-mode'.
> ^ C-c C-t t
> 
> +package of the current buffer. It is bound to 'C-c C-p' in 'go-ts-mode'.
> ^ C-c C-t p
> 
> + "List of go build tags for the test commands."
> ^ Go
> 
> + "Return a list with names of all defuns in the range."
> We should probably say what the range actually is (START to END) - not sure if we have a convention for that wording already.

A few more nits:
+  "Return compile flag for build tags.
          ^ the

+This function respects `go-ts-mode-build-tags' variable for specifying
                       ^ the

+  "Return a list with names of all defuns in the range."
                      ^ the

Indentation is also off in go-ts-mode-test-function-at-point.

When we run C-c C-t t outside of a function, we get:
go test -v  -run '^nil$'
Should we maybe not bother running anything at all? What do you think?
Do we know how other packages behave under similar circumstances?





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-28  2:30                 ` Randy Taylor
@ 2024-05-28 19:58                   ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-19 18:17                     ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 23+ messages in thread
From: Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-28 19:58 UTC (permalink / raw)
  To: Randy Taylor; +Cc: Eli Zaretskii, 70939

> > Sorry for the delay in reviewing, I've been having internet troubles since last Friday. I'll take a look at this next week.

No issues, appreciate the review.

> > A few quick things I noticed on a glance:
> >
> > + (let* ((node (go-ts-mode--find-defun-at start))
> > + (name (treesit-defun-name node))
> >
> > Indentation is off on the name line - looks like a TAB was used? Should only be spaces everywhere. Double check the rest is OK.
> >
> > +region. It is bound to 'C-c C-t' in 'go-ts-mode'.
> > ^ C-c C-t t
> >
> > +package of the current buffer. It is bound to 'C-c C-p' in 'go-ts-mode'.
> > ^ C-c C-t p
> >
> > + "List of go build tags for the test commands."
> > ^ Go
> >
> > + "Return a list with names of all defuns in the range."
> > We should probably say what the range actually is (START to END) - not sure if we have a convention for that wording already.
>
> A few more nits:
> +  "Return compile flag for build tags.
>           ^ the
>
> +This function respects `go-ts-mode-build-tags' variable for specifying
>                        ^ the
>
> +  "Return a list with names of all defuns in the range."
>                       ^ the
>
> Indentation is also off in go-ts-mode-test-function-at-point.

Thanks. I learned about whitespace-mode and checkdoc. I'll be running my changes
through it before sending the path now. Please let me know if there are any
other checks I can do in the future.

> When we run C-c C-t t outside of a function, we get:
> go test -v  -run '^nil$'
> Should we maybe not bother running anything at all? What do you think?
> Do we know how other packages behave under similar circumstances?

I'll check the packages and find out how it is handled elsewhere. One point of
reference can be Doom Emacs configuration. It uses the
re-search-(backward|forward) functions that throw an error if no match is found.

I'm also planning to add a third function to run all the tests in the current
file using the buffer-beginning and buffer-end as the range. I'll submit it in
the next patch along with the suggested fixes.

-- 
Ankit





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-05-28 19:58                   ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-06-19 18:17                     ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-21  2:40                       ` Randy Taylor
  0 siblings, 1 reply; 23+ messages in thread
From: Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-19 18:17 UTC (permalink / raw)
  To: Randy Taylor; +Cc: Eli Zaretskii, 70939

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

Apologies for the delay in response.

> > > A few quick things I noticed on a glance:
> > >
> > > + (let* ((node (go-ts-mode--find-defun-at start))
> > > + (name (treesit-defun-name node))
> > >
> > > Indentation is off on the name line - looks like a TAB was used? Should only be spaces everywhere. Double check the rest is OK.
> > >
> > > +region. It is bound to 'C-c C-t' in 'go-ts-mode'.
> > > ^ C-c C-t t
> > >
> > > +package of the current buffer. It is bound to 'C-c C-p' in 'go-ts-mode'.
> > > ^ C-c C-t p
> > >
> > > + "List of go build tags for the test commands."
> > > ^ Go
> > >
> > > + "Return a list with names of all defuns in the range."
> > > We should probably say what the range actually is (START to END) - not sure if we have a convention for that wording already.
> >
> > A few more nits:
> > +  "Return compile flag for build tags.
> >           ^ the
> >
> > +This function respects `go-ts-mode-build-tags' variable for specifying
> >                        ^ the
> >
> > +  "Return a list with names of all defuns in the range."
> >                       ^ the
> >
> > Indentation is also off in go-ts-mode-test-function-at-point.
>
> Thanks. I learned about whitespace-mode and checkdoc. I'll be running my changes
> through it before sending the path now. Please let me know if there are any
> other checks I can do in the future.

I've incorporated these suggestions in my updated patch.

> > When we run C-c C-t t outside of a function, we get:
> > go test -v  -run '^nil$'
> > Should we maybe not bother running anything at all? What do you think?
> > Do we know how other packages behave under similar circumstances?
>
> I'll check the packages and find out how it is handled elsewhere. One point of
> reference can be Doom Emacs configuration. It uses the
> re-search-(backward|forward) functions that throw an error if no match is found.

I've updated the functions to now raise an error if no function is found at
point or under the region.

> I'm also planning to add a third function to run all the tests in the current
> file using the buffer-beginning and buffer-end as the range. I'll submit it in
> the next patch along with the suggested fixes.

I've added the go-ts-mode-test-file function as well that runs all the unit
tests in the current file.

--
Ankit

[-- Attachment #2: 0001-Add-commands-to-run-unit-tests-in-go-ts-mode.patch --]
[-- Type: text/x-patch, Size: 6461 bytes --]

From c7bb51491c59273fdc0b2ee0ef38513007d7eb70 Mon Sep 17 00:00:00 2001
From: Ankit R Gadiya <git@argp.in>
Date: Tue, 14 May 2024 00:14:03 +0530
Subject: [PATCH] Add commands to run unit tests in go-ts-mode

Three new commands are added in the go-ts-mode to run unit tests.
The go-ts-mode-test-function-at-point command runs the current
test function at point or all the functions in the active region
when region is active.  It is bound to "C-c C-t t". The
go-ts-mode-test-file command runs all tests in the current file. It is
bound to "C-c C-t f". The go-ts-mode-test-package command runs all tests
in the current package.  It is bound to "C-c C-t p".

* lisp/progmodes/go-ts-mode.el (go-ts-mode-build-tags): New variable.
(go-ts-mode-map): New map variable.
(go-ts-mode--get-build-tags-flag): New function.
(go-ts-mode--compile-test): New function.
(go-ts-mode--find-defun-at): New function.
(go-ts-mode--get-function-regexp): New function.
(go-ts-mode--get-functions-in-range): New function.
(go-ts-mode--get-test-regexp-at-point): New function.
(go-ts-mode-test-function-at-point): New function.
(go-ts-mode-test-file): New function.
(go-ts-mode-test-package): New function.
* etc/NEWS: Mention the change.

(Bug#70939)
---
 etc/NEWS                     | 16 +++++++
 lisp/progmodes/go-ts-mode.el | 84 +++++++++++++++++++++++++++++++++++-
 2 files changed, 99 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 3b18972860f..73fcb46fad2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1389,6 +1389,22 @@ This command adds a docstring comment to the current defun.  If a
 comment already exists, point is only moved to the comment.  It is
 bound to 'C-c C-d' in 'go-ts-mode'.
 
+*** New unit test commands.
+Two new commands are now available to run unit tests.
+
+The 'go-ts-mode-test-function-at-point' command runs the unit test at
+point.  If a region is active, it runs all the unit tests under the
+region.  It is bound to 'C-c C-t t' in 'go-ts-mode'.
+
+The 'go-ts-mode-test-file' command runs all unit tests in the current
+file. It is bound to 'C-c C-t f' in 'go-ts-mode'.
+
+The 'go-ts-mode-test-package' command runs all unit tests under the
+package of the current buffer.  It is bound to 'C-c C-t p' in 'go-ts-mode'.
+
+The 'go-ts-mode-build-tags' variable is available to set a list of build
+tags for the test commands.
+
 ** Man mode
 
 +++
diff --git a/lisp/progmodes/go-ts-mode.el b/lisp/progmodes/go-ts-mode.el
index 2d3e6aac090..b27742a0785 100644
--- a/lisp/progmodes/go-ts-mode.el
+++ b/lisp/progmodes/go-ts-mode.el
@@ -46,6 +46,12 @@ go-ts-mode-indent-offset
   :safe 'integerp
   :group 'go)
 
+(defcustom go-ts-mode-build-tags nil
+  "List of Go build tags for the test commands."
+  :version "30.1"
+  :type '(repeat string)
+  :group 'go)
+
 (defvar go-ts-mode--syntax-table
   (let ((table (make-syntax-table)))
     (modify-syntax-entry ?+   "."      table)
@@ -242,7 +248,10 @@ go-ts-mode--font-lock-settings
 (defvar-keymap go-ts-mode-map
   :doc "Keymap used in Go mode, powered by tree-sitter"
   :parent prog-mode-map
-  "C-c C-d" #'go-ts-mode-docstring)
+  "C-c C-d" #'go-ts-mode-docstring
+  "C-c C-t t" #'go-ts-mode-test-function-at-point
+  "C-c C-t f" #'go-ts-mode-test-file
+  "C-c C-t p" #'go-ts-mode-test-package)
 
 ;;;###autoload
 (define-derived-mode go-ts-mode prog-mode "Go"
@@ -375,6 +384,79 @@ go-ts-mode--comment-on-previous-line-p
      (<= (treesit-node-start node) point (treesit-node-end node))
      (string-equal "comment" (treesit-node-type node)))))
 
+(defun go-ts-mode--get-build-tags-flag ()
+  "Return the compile flag for build tags.
+This function respects the `go-ts-mode-build-tags' variable for
+specifying build tags."
+  (if go-ts-mode-build-tags
+      (format "-tags %s" (string-join go-ts-mode-build-tags ","))
+    ""))
+
+(defun go-ts-mode--compile-test (regexp)
+  "Compile the tests matching REGEXP.
+This function respects the `go-ts-mode-build-tags' variable for
+specifying build tags."
+  (compile (format "go test -v %s -run '%s'"
+                   (go-ts-mode--get-build-tags-flag)
+                   regexp)))
+
+(defun go-ts-mode--find-defun-at (start)
+  "Return the first defun node from START."
+  (let ((thing (or treesit-defun-type-regexp 'defun)))
+    (or (treesit-thing-at start thing)
+        (treesit-thing-next start thing))))
+
+(defun go-ts-mode--get-function-regexp (name)
+  (if name
+      (format "^%s$" name)
+    (error "No test function found")))
+
+(defun go-ts-mode--get-functions-in-range (start end)
+  "Return a list with the names of all defuns in the range START to END."
+  (let* ((node (go-ts-mode--find-defun-at start))
+         (name (treesit-defun-name node))
+         (node-start (treesit-node-start node))
+         (node-end (treesit-node-end node)))
+    (if (or (not node)
+            (> start node-end)
+            (< end node-start))
+        nil
+      (cons (go-ts-mode--get-function-regexp name)
+            (go-ts-mode--get-functions-in-range (treesit-node-end node) end)))))
+
+(defun go-ts-mode--get-test-regexp-at-point ()
+  "Return a regular expression for tests at point.
+If region is active, the regexp will include all the functions under the
+region."
+  (if (region-active-p)
+      (string-join (go-ts-mode--get-functions-in-range (region-beginning)
+                                                       (region-end))
+                   "|")
+    (go-ts-mode--get-function-regexp (treesit-defun-name (treesit-defun-at-point)))))
+
+(defun go-ts-mode-test-function-at-point ()
+  "Run the unit test at point.
+If the point is anywhere in the test function, that function will be
+run.  If the region is selected, all the functions under the region will
+be run."
+    (interactive)
+    (go-ts-mode--compile-test (go-ts-mode--get-test-regexp-at-point)))
+
+(defun go-ts-mode-test-file ()
+  "Run all the unit tests in the current file."
+  (interactive)
+  (let ((defuns (go-ts-mode--get-functions-in-range (point-min) (point-max))))
+    (if defuns
+        (go-ts-mode--compile-test (string-join defuns "|"))
+      (error "No test functions found in the current file"))))
+
+(defun go-ts-mode-test-package ()
+  "Run all the unit tests under the current package."
+  (interactive)
+  (compile (format "go test -v %s -run %s"
+                   (go-ts-mode--get-build-tags-flag)
+                   default-directory)))
+
 ;; go.mod support.
 
 (defvar go-mod-ts-mode--syntax-table
-- 
2.39.2


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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-06-19 18:17                     ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-06-21  2:40                       ` Randy Taylor
  2024-06-23 14:46                         ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-23 14:56                         ` Eli Zaretskii
  0 siblings, 2 replies; 23+ messages in thread
From: Randy Taylor @ 2024-06-21  2:40 UTC (permalink / raw)
  To: Ankit Gadiya; +Cc: Eli Zaretskii, 70939

On Wednesday, June 19th, 2024 at 14:17, Ankit Gadiya via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> wrote:
> 
> 
> Apologies for the delay in response.
> 
> > > > A few quick things I noticed on a glance:
> > > > 
> > > > + (let* ((node (go-ts-mode--find-defun-at start))
> > > > + (name (treesit-defun-name node))
> > > > 
> > > > Indentation is off on the name line - looks like a TAB was used? Should only be spaces everywhere. Double check the rest is OK.
> > > > 
> > > > +region. It is bound to 'C-c C-t' in 'go-ts-mode'.
> > > > ^ C-c C-t t
> > > > 
> > > > +package of the current buffer. It is bound to 'C-c C-p' in 'go-ts-mode'.
> > > > ^ C-c C-t p
> > > > 
> > > > + "List of go build tags for the test commands."
> > > > ^ Go
> > > > 
> > > > + "Return a list with names of all defuns in the range."
> > > > We should probably say what the range actually is (START to END) - not sure if we have a convention for that wording already.
> > > 
> > > A few more nits:
> > > + "Return compile flag for build tags.
> > > ^ the
> > > 
> > > +This function respects `go-ts-mode-build-tags' variable for specifying
> > > ^ the
> > > 
> > > + "Return a list with names of all defuns in the range."
> > > ^ the
> > > 
> > > Indentation is also off in go-ts-mode-test-function-at-point.
> > 
> > Thanks. I learned about whitespace-mode and checkdoc. I'll be running my changes
> > through it before sending the path now. Please let me know if there are any
> > other checks I can do in the future.
> 
> 
> I've incorporated these suggestions in my updated patch.
> 
> > > When we run C-c C-t t outside of a function, we get:
> > > go test -v -run '^nil$'
> > > Should we maybe not bother running anything at all? What do you think?
> > > Do we know how other packages behave under similar circumstances?
> > 
> > I'll check the packages and find out how it is handled elsewhere. One point of
> > reference can be Doom Emacs configuration. It uses the
> > re-search-(backward|forward) functions that throw an error if no match is found.
> 
> 
> I've updated the functions to now raise an error if no function is found at
> point or under the region.
> 
> > I'm also planning to add a third function to run all the tests in the current
> > file using the buffer-beginning and buffer-end as the range. I'll submit it in
> > the next patch along with the suggested fixes.
> 
> 
> I've added the go-ts-mode-test-file function as well that runs all the unit
> tests in the current file.
> 
> --
> Ankit

When running go-ts-mode-test-file, it seems to match things that aren't functions like interfaces. I think we should only be matching functions, and more specifically, shouldn't we only be matching functions starting with "Test"?
We could perhaps extend the error checking to include that as well?

For the commit message, I'm not sure we need that paragraph especially when it's already described in the news. Eli what do you think?

+*** New unit test commands.
+Two new commands are now available to run unit tests.
Three?

I'm also wondering if we should include "current" in the go-ts-mode-test-file and go-ts-mode-test-package function names. Maybe someone would expect that they would get prompted to select a file or package to test? Maybe I'm overthinking that :). Eli what do you think?





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-06-21  2:40                       ` Randy Taylor
@ 2024-06-23 14:46                         ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-26  2:26                           ` Randy Taylor
  2024-06-23 14:56                         ` Eli Zaretskii
  1 sibling, 1 reply; 23+ messages in thread
From: Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-23 14:46 UTC (permalink / raw)
  To: Randy Taylor; +Cc: Eli Zaretskii, 70939

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

> When running go-ts-mode-test-file, it seems to match things that aren't functions like interfaces. I think we should only be matching functions, and more specifically, shouldn't we only be matching functions starting with "Test"?
> We could perhaps extend the error checking to include that as well?

I've updated the patch to include a "function_declaration" and "Test" prefix
check. I also did a minor refactoring to avoid special handling in the case when
the region is not active. I'm also attaching the updated sample file with more
scenarios for testing.

>
> For the commit message, I'm not sure we need that paragraph especially when it's already described in the news. Eli what do you think?
>
> +*** New unit test commands.
> +Two new commands are now available to run unit tests.
> Three?
>
> I'm also wondering if we should include "current" in the go-ts-mode-test-file and go-ts-mode-test-package function names. Maybe someone would expect that they would get prompted to select a file or package to test? Maybe I'm overthinking that :). Eli what do you think?

I'll wait for Eli to reply before incorporating the changes :).

Additionally, I also noticed that the emacs-30 branch is cut. I wanted
to check if I
need to rebase my patch onto master or emacs-30 branch?

--
Ankit

[-- Attachment #2: 0001-Add-commands-to-run-unit-tests-in-go-ts-mode.patch --]
[-- Type: text/x-patch, Size: 6727 bytes --]

From 002215f2ee94252edd66908963a66b01f62c36d1 Mon Sep 17 00:00:00 2001
From: Ankit R Gadiya <git@argp.in>
Date: Tue, 14 May 2024 00:14:03 +0530
Subject: [PATCH] Add commands to run unit tests in go-ts-mode

Three new commands are added in the go-ts-mode to run unit tests.
The go-ts-mode-test-function-at-point command runs the current
test function at point or all the functions in the active region
when region is active.  It is bound to "C-c C-t t". The
go-ts-mode-test-file command runs all tests in the current file. It is
bound to "C-c C-t f". The go-ts-mode-test-package command runs all tests
in the current package.  It is bound to "C-c C-t p".

* lisp/progmodes/go-ts-mode.el (go-ts-mode-build-tags): New variable.
(go-ts-mode-map): New map variable.
(go-ts-mode--get-build-tags-flag): New function.
(go-ts-mode--compile-test): New function.
(go-ts-mode--find-defun-at): New function.
(go-ts-mode--get-function-regexp): New function.
(go-ts-mode--get-functions-in-range): New function.
(go-ts-mode--get-test-regexp-at-point): New function.
(go-ts-mode-test-function-at-point): New function.
(go-ts-mode-test-file): New function.
(go-ts-mode-test-package): New function.
* etc/NEWS: Mention the change.

(Bug#70939)
---
 etc/NEWS                     | 16 +++++++
 lisp/progmodes/go-ts-mode.el | 90 +++++++++++++++++++++++++++++++++++-
 2 files changed, 105 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 3b18972860f..73fcb46fad2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1389,6 +1389,22 @@ This command adds a docstring comment to the current defun.  If a
 comment already exists, point is only moved to the comment.  It is
 bound to 'C-c C-d' in 'go-ts-mode'.
 
+*** New unit test commands.
+Two new commands are now available to run unit tests.
+
+The 'go-ts-mode-test-function-at-point' command runs the unit test at
+point.  If a region is active, it runs all the unit tests under the
+region.  It is bound to 'C-c C-t t' in 'go-ts-mode'.
+
+The 'go-ts-mode-test-file' command runs all unit tests in the current
+file. It is bound to 'C-c C-t f' in 'go-ts-mode'.
+
+The 'go-ts-mode-test-package' command runs all unit tests under the
+package of the current buffer.  It is bound to 'C-c C-t p' in 'go-ts-mode'.
+
+The 'go-ts-mode-build-tags' variable is available to set a list of build
+tags for the test commands.
+
 ** Man mode
 
 +++
diff --git a/lisp/progmodes/go-ts-mode.el b/lisp/progmodes/go-ts-mode.el
index 2d3e6aac090..8d23b230c3f 100644
--- a/lisp/progmodes/go-ts-mode.el
+++ b/lisp/progmodes/go-ts-mode.el
@@ -46,6 +46,12 @@ go-ts-mode-indent-offset
   :safe 'integerp
   :group 'go)
 
+(defcustom go-ts-mode-build-tags nil
+  "List of Go build tags for the test commands."
+  :version "30.1"
+  :type '(repeat string)
+  :group 'go)
+
 (defvar go-ts-mode--syntax-table
   (let ((table (make-syntax-table)))
     (modify-syntax-entry ?+   "."      table)
@@ -242,7 +248,10 @@ go-ts-mode--font-lock-settings
 (defvar-keymap go-ts-mode-map
   :doc "Keymap used in Go mode, powered by tree-sitter"
   :parent prog-mode-map
-  "C-c C-d" #'go-ts-mode-docstring)
+  "C-c C-d" #'go-ts-mode-docstring
+  "C-c C-t t" #'go-ts-mode-test-function-at-point
+  "C-c C-t f" #'go-ts-mode-test-file
+  "C-c C-t p" #'go-ts-mode-test-package)
 
 ;;;###autoload
 (define-derived-mode go-ts-mode prog-mode "Go"
@@ -375,6 +384,85 @@ go-ts-mode--comment-on-previous-line-p
      (<= (treesit-node-start node) point (treesit-node-end node))
      (string-equal "comment" (treesit-node-type node)))))
 
+(defun go-ts-mode--get-build-tags-flag ()
+  "Return the compile flag for build tags.
+This function respects the `go-ts-mode-build-tags' variable for
+specifying build tags."
+  (if go-ts-mode-build-tags
+      (format "-tags %s" (string-join go-ts-mode-build-tags ","))
+    ""))
+
+(defun go-ts-mode--compile-test (regexp)
+  "Compile the tests matching REGEXP.
+This function respects the `go-ts-mode-build-tags' variable for
+specifying build tags."
+  (compile (format "go test -v %s -run '%s'"
+                   (go-ts-mode--get-build-tags-flag)
+                   regexp)))
+
+(defun go-ts-mode--find-defun-at (start)
+  "Return the first defun node from START."
+  (let ((thing (or treesit-defun-type-regexp 'defun)))
+    (or (treesit-thing-at start thing)
+        (treesit-thing-next start thing))))
+
+(defun go-ts-mode--get-function-regexp (name)
+  (if name
+      (format "^%s$" name)
+    (error "No test function found")))
+
+(defun go-ts-mode--get-functions-in-range (start end)
+  "Return a list with the names of all defuns in the range START to END."
+  (let* ((node (go-ts-mode--find-defun-at start))
+         (name (treesit-defun-name node))
+         (node-start (treesit-node-start node))
+         (node-end (treesit-node-end node)))
+    (cond ((or (not node)
+               (> start node-end)
+               (< end node-start))
+           nil)
+          ((or (not (equal (treesit-node-type node) "function_declaration"))
+               (not (string-prefix-p "Test" name)))
+           (go-ts-mode--get-functions-in-range (treesit-node-end node) end))
+          (t
+           (cons (go-ts-mode--get-function-regexp name)
+                 (go-ts-mode--get-functions-in-range (treesit-node-end node) end))))))
+
+(defun go-ts-mode--get-test-regexp-at-point ()
+  "Return a regular expression for tests at point.
+If region is active, the regexp will include all the functions under the
+region."
+  (let* ((range (if (region-active-p)
+                    (list (region-beginning) (region-end))
+                  (list (point) (point))))
+         (funcs (apply #'go-ts-mode--get-functions-in-range range)))
+    (if funcs
+      (string-join funcs "|")
+    (error "No test function found"))))
+
+(defun go-ts-mode-test-function-at-point ()
+  "Run the unit test at point.
+If the point is anywhere in the test function, that function will be
+run.  If the region is selected, all the functions under the region will
+be run."
+    (interactive)
+    (go-ts-mode--compile-test (go-ts-mode--get-test-regexp-at-point)))
+
+(defun go-ts-mode-test-file ()
+  "Run all the unit tests in the current file."
+  (interactive)
+  (let ((defuns (go-ts-mode--get-functions-in-range (point-min) (point-max))))
+    (if defuns
+        (go-ts-mode--compile-test (string-join defuns "|"))
+      (error "No test functions found in the current file"))))
+
+(defun go-ts-mode-test-package ()
+  "Run all the unit tests under the current package."
+  (interactive)
+  (compile (format "go test -v %s -run %s"
+                   (go-ts-mode--get-build-tags-flag)
+                   default-directory)))
+
 ;; go.mod support.
 
 (defvar go-mod-ts-mode--syntax-table
-- 
2.39.2


[-- Attachment #3: sample_test.go --]
[-- Type: text/x-go, Size: 319 bytes --]

package sample_test

import "testing"

type Foo interface{}

type Bar struct{}

func (b *Bar) Method() {
}

func TestIndividual(t *testing.T) {
}

func TestIndividualFooBar(t *testing.T) {
}

func TestOverlapRegion(t *testing.T) {
}

func testHelper() {}

func nonTest() {}

func TestNewOverlapRegion(t *testing.T) {
}

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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-06-21  2:40                       ` Randy Taylor
  2024-06-23 14:46                         ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-06-23 14:56                         ` Eli Zaretskii
  1 sibling, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2024-06-23 14:56 UTC (permalink / raw)
  To: Randy Taylor; +Cc: ankit, 70939

> Date: Fri, 21 Jun 2024 02:40:12 +0000
> From: Randy Taylor <dev@rjt.dev>
> Cc: Eli Zaretskii <eliz@gnu.org>, 70939@debbugs.gnu.org
> 
> For the commit message, I'm not sure we need that paragraph especially when it's already described in the news. Eli what do you think?

Yes, the commit log doesn't need to say again what is already in NEWS,
it can be shorter.

> I'm also wondering if we should include "current" in the go-ts-mode-test-file and go-ts-mode-test-package function names. Maybe someone would expect that they would get prompted to select a file or package to test? Maybe I'm overthinking that :). Eli what do you think?

I'd use "this" instead of "current".





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

* bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
  2024-06-23 14:46                         ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-06-26  2:26                           ` Randy Taylor
  0 siblings, 0 replies; 23+ messages in thread
From: Randy Taylor @ 2024-06-26  2:26 UTC (permalink / raw)
  To: Ankit Gadiya; +Cc: Eli Zaretskii, 70939

On Sunday, June 23rd, 2024 at 10:46, Ankit Gadiya <ankit@argp.in> wrote:
> 
> 
> > When running go-ts-mode-test-file, it seems to match things that aren't functions like interfaces. I think we should only be matching functions, and more specifically, shouldn't we only be matching functions starting with "Test"?
> 
> > We could perhaps extend the error checking to include that as well?
> 
> 
> I've updated the patch to include a "function_declaration" and "Test" prefix
> check. I also did a minor refactoring to avoid special handling in the case when
> the region is not active. I'm also attaching the updated sample file with more
> scenarios for testing.

Thanks, the changes look good to me.

> 
> > For the commit message, I'm not sure we need that paragraph especially when it's already described in the news. Eli what do you think?
> > 
> > +*** New unit test commands.
> > +Two new commands are now available to run unit tests.
> > Three?

This still needs to be updated.

A few more comments:
+(defun go-ts-mode--get-test-regexp-at-point ()
+  "Return a regular expression for tests at point.
                                   ^ the

Could go-ts-mode--get-test-regexp-at-point and go-ts-mode-test-file use if-let?

Also, the indentation looks off in go-ts-mode-test-function-at-point (2 extra spaces methinks).

> > 
> > I'm also wondering if we should include "current" in the go-ts-mode-test-file and go-ts-mode-test-package function names. Maybe someone would expect that they would get prompted to select a file or package to test? Maybe I'm overthinking that :). Eli what do you think?
> 
> 
> I'll wait for Eli to reply before incorporating the changes :).

And he chimed in - let's go with his suggestions.

> 
> Additionally, I also noticed that the emacs-30 branch is cut. I wanted
> to check if I
> need to rebase my patch onto master or emacs-30 branch?

I would guess master, but let's see what Eli says.

> 
> --
> Ankit





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

end of thread, other threads:[~2024-06-26  2:26 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-14 14:04 bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-14 16:52 ` Eli Zaretskii
2024-05-14 17:24   ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-14 17:59     ` Eli Zaretskii
2024-05-15  2:36 ` Randy Taylor
2024-05-15  4:55   ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-16  2:32     ` Randy Taylor
2024-05-16  8:27       ` Eli Zaretskii
2024-05-16 15:03         ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-16 16:01           ` Eli Zaretskii
2024-05-18  9:54             ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-25  2:35               ` Randy Taylor
2024-05-28  2:30                 ` Randy Taylor
2024-05-28 19:58                   ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-19 18:17                     ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-21  2:40                       ` Randy Taylor
2024-06-23 14:46                         ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-26  2:26                           ` Randy Taylor
2024-06-23 14:56                         ` Eli Zaretskii
2024-05-17  2:27       ` Randy Taylor
2024-05-18  8:55         ` Ankit Gadiya via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-15 11:21   ` Eli Zaretskii
2024-05-16  1:24     ` Randy Taylor

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.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).