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: 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@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?





reply via email to

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