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

Previous Next

Full log

Message #62 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: Fri, 21 Jun 2024 02:40:12 +0000

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 <at> 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?

Previous Next

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