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

Previous Next

Full log

Message #71 received at 70939 <at> debbugs.gnu.org (full text, mbox):

From: Randy Taylor <dev <at> rjt.dev>
To: Ankit Gadiya <ankit <at> argp.in>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 70939 <at> debbugs.gnu.org
Subject: Re: 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 <at> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.