bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode


From: Randy Taylor
Subject: bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode
Date: Wed, 26 Jun 2024 02:26:35 +0000

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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]