[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