Support running single test in a testify suite #25

jstensland · 2024-06-05T16:54:12Z

When running the nearest test in a suite, I would expect the run command to only run the specified test, but instead it quietly misses completely.

Expected run spec command -run option

go test ./path/... -run '^TestSuiteName/TestName$'

Actual run spec command -run option

go test ./path/... -run '^TestName$'

The additional surprise here is that when then actual command is run, there is no notification that no tests were matched, the summary suggested it worked, but the coverage wasn't updated.

This is when running the nearest test, looking at the run spec produced by build_single_test_runspec

The text was updated successfully, but these errors were encountered:

fredrikaverpil · 2024-06-05T19:13:15Z

Hm 🤔, I'm wondering what you are referring to when you say "suite".

Neotest has a notion to run all tests of a project, which I believe is what is called a "suite". But it kind of sounds like you mean something very specific here (and not just all test functions of a project)?

Exactly what is TestSuiteName on your end, if it is not a test function?

thelooter · 2024-06-05T21:28:08Z

I think this is related to testify-suites

https://pkg.go.dev/github.com/stretchr/testify/suite

jstensland · 2024-06-05T23:17:18Z

Yes, apologies, I mean a testify suite, or something similar.

fredrikaverpil · 2024-06-06T07:52:58Z

Ok, got it, thanks for this @jstensland and @thelooter !

I'm not sure, by just looking at the example below (from the testify docs), what could be the best way to attempt to initially support this.

// Basic imports
import (
    "testing"
    "github.com/stretchr/testify/assert"
    "github.com/stretchr/testify/suite"
)

// Define the suite, and absorb the built-in basic suite
// functionality from testify - including a T() method which
// returns the current testing context
type ExampleTestSuite struct {
    suite.Suite
    VariableThatShouldStartAtFive int
}

// Make sure that VariableThatShouldStartAtFive is set to five
// before each test
func (suite *ExampleTestSuite) SetupTest() {
    suite.VariableThatShouldStartAtFive = 5
}

// All methods that begin with "Test" are run as tests within a
// suite.
func (suite *ExampleTestSuite) TestExample() {
    assert.Equal(suite.T(), 5, suite.VariableThatShouldStartAtFive)
    suite.Equal(5, suite.VariableThatShouldStartAtFive)
}

// In order for 'go test' to run this suite, we need to create
// a normal test function and pass our suite to suite.Run
func TestExampleTestSuite(t *testing.T) {
    suite.Run(t, new(ExampleTestSuite))
}

I haven't used testify personally, but I would like to look into how it works by setting up a testify suite and play around with it. My gut feeling is though that this could lead to having to redesign the adapter into being able to handle multiple "modes" or similar, and the detection of said modes. Today we have testify, tomorrow we might have something different. Could get ugly if you mix them... I'm also wondering how far users are taking it with these testify suites. The more crazy you get with e.g. the receiver signature, the harder it could be to do the right thing and have bugs.

Anyway, I'm mostly thinking out loud here. Would love any of your input/insights into this if you have any 😄 so I can take better decisions here on trying to support testify.

@jstensland I'm away from my laptop until next week, but if I read this correctly, one solution would be to tweak the test detection and have all testify tests prefixed by its suite name. Meaning, instead of listing TestExample, you would see ExampleTestSuite/TestExample in the Neotest test summary window. I don't believe that Neotest has a notion of "groups" or "suites" in this sense that could be leveraged. Would this theory hold up and scale?

Also, right now, I believe TestExampleTestSuite will be picked up as a regular test, and would be run if you e.g. executed all tests in a file. This would result in that you first run all tests individually and finally this one would get run (which runs all the tests again). Not really sure how to reliably disable this test from when you e.g. run all tests of a file or all tests underneath a folder.

fredrikaverpil · 2024-06-06T08:13:50Z

I'm just leaving it here for now, but there are likely a lot of good insights posted on the neotest-go adapter:

Issue, about 2 years old: [Feature request] Support for Testify framework nvim-neotest/neotest-go#6
PR, about 6 months old: Support test under cursor for testify suites, support . in file names nvim-neotest/neotest-go#67 however, I'm wary of requiring specific naming conventions.

But like I said, I would like to play around with this all and find a maintainable and robust solution.

jstensland · 2024-06-06T16:51:59Z

Starting with an example to try to make my statements more concrete. using that example testify suite with some slight modifications, say you add this to your test file testname_test.go

// Define the example testify suite
type exampleSuite struct {
	suite.Suite
	VariableThatShouldStartAtFive int
}

func (suite *exampleSuite) SetupTest() {
	suite.VariableThatShouldStartAtFive = 5
}

func (suite *exampleSuite) TestSuiteTest1() {
	suite.Equal(5, suite.VariableThatShouldStartAtFive)
}

func (suite *exampleSuite) TestSuiteTest2() {
	suite.Equal(5, suite.VariableThatShouldStartAtFive)
}

// Where go detects the test
func TestExampleSuite(t *testing.T) {
	suite.Run(t, new(exampleSuite))
}

Go test output is like this

▶ go test -v
=== RUN   TestAdd
--- PASS: TestAdd (0.00s)
=== RUN   TestNames
=== RUN   TestNames/Mixed_case_with_space
=== RUN   TestNames/Comma_,_and_'_are_ok_to_use
=== RUN   TestNames/Brackets_[1]_(2)_{3}_are_ok
--- PASS: TestNames (0.00s)
    --- PASS: TestNames/Mixed_case_with_space (0.00s)
    --- PASS: TestNames/Comma_,_and_'_are_ok_to_use (0.00s)
    --- PASS: TestNames/Brackets_[1]_(2)_{3}_are_ok (0.00s)
=== RUN   TestExampleSuite
=== RUN   TestExampleSuite/TestSuiteTest1
=== RUN   TestExampleSuite/TestSuiteTest2
--- PASS: TestExampleSuite (0.00s)
    --- PASS: TestExampleSuite/TestSuiteTest1 (0.00s)
    --- PASS: TestExampleSuite/TestSuiteTest2 (0.00s)
PASS
ok      github.com/fredrikaverpil/neotest-golang        0.304s

you can target a particular suite test with a -run regex like this

▶ go test ./... -run '^TestExampleSuite/TestSuiteTest2$' -v
=== RUN   TestExampleSuite
=== RUN   TestExampleSuite/TestSuiteTest2
--- PASS: TestExampleSuite (0.00s)
    --- PASS: TestExampleSuite/TestSuiteTest2 (0.00s)
PASS
ok      github.com/fredrikaverpil/neotest-golang        0.261s

or with flags to testify like this

▶ go test ./... -run TestExampleSuite -testify.m  TestSuiteTest2 -v 
=== RUN   TestExampleSuite
=== RUN   TestExampleSuite/TestSuiteTest2
--- PASS: TestExampleSuite (0.00s)
    --- PASS: TestExampleSuite/TestSuiteTest2 (0.00s)
PASS
ok      github.com/fredrikaverpil/neotest-golang        0.315s

The former approach seems more general to me, and is where I started

You're right that TestExampleTestSuite will be picked up as a regular test, and that's what testify depends on too. go test starts there.

This would result in that you first run all tests individually and finally this one would get run

This is only an issue because you're running the entire package test by test, right? Once packages are run all at once, you wouldn't have the double invocation. go test PATH/TO/PKG only runs things once.

Those are the most relevant threads I have found. I see you have this code as well, which is why they get detected but the name is only the method, and does not include the Suite the way the go test output does.

I don't believe that Neotest has a notion of "groups" or "suites" in this sense that could be leveraged.

This is where I stopped, since I don't know the neotest framework well. Can you not create a namespace for all the suite tests some how? I haven't figured out how the treesitter tags get interpreted yet, when discovering test positions. From a user point of view, I would expect the methods to be grouped under the suite, similar to test cases.

Other thoughts

I also do not like the idea of detecting based on names
testify suites and plain go tests will definitely live in the same package. If testify is opt-in, that option could work, but an exclusively testify mode would not be helpful IMO
I also see both list and map test cases get used in suite methods as well

uroborosq · 2024-06-19T19:52:37Z

Hello everyone,

I've investigated a bit this problem and want to share some results.

The branch is here https://github.com/uroborosq/neotest-golang/tree/feat/testify_suites

For now it's only extending treesitter query. I've add two like subqueries - first collect suite methods into namespace with name of standard go test function and second collects directly suite methods.
These names adds to position id which lets tests and subtests runs with go test -run ...

So generally it works, I can run and debug suite method/subtests, but there is a list of limitations:

if parent method declaration and suite methods are in different files, it won't work. haven't investigated yet, but this line concerns a bit - https://github.com/nvim-neotest/neotest/blob/master/lua/neotest/lib/treesitter/init.lua#L48.
It's necessary to parse whole dir to do it correctly, but it seems like it can be parsed only by files from at first sight. Although there is a possibility to run all tests in project, from impression of the code of neotest (some tricks with merging trees) parsing still seems to be per-file.
There is a problem that in my implementation there is no one namespace for all suite's methods - for each method creates it own namespace. All of them have the same name. Looks like this:

So TestProductService is a parent method and it appears three times - first as separate test function (can be hided I think with just queres), next two as namespace for suite methods. It doesn't look like a problem to merge after parsing, but there is one obstacle with this 'test' structure of which neotest tree is consisted.
It contains 'range' field with range of definition of test or namespace and it is only one range for test/namespace, but in case of go and testify suites it ideally has to be list of ranges. So, for each method it will be a range and everyone belong to one namespace.
Besides feature request to neotest to provide multiple ranges, I see two more options:
assign one wide range to the suite namespace, which will cover all methods (with #make-range directive for treesitter or custom build_position implementation for neotest). However, I think, that if somebody put ordinary test function between suite methods, neotest will consider it belonging to suite namespace and it will be impossible to run it.
the another option is to not use namespaces, may be add some prefix to test name of suite method and provide them as plain list. I think it is better approach, but still need to add some markers in treesitter query and then parse them in custom build_position function. Haven't tried yet, but looks possible

Mb there are some other possibilites, I am very new to lua and neovim)

fredrikaverpil · 2024-06-25T20:49:40Z

Thanks for having a look at this @uroborosq 😄
You queries looks quite advanced and I can't get them all to work together in the query editor (:EditQuery). I can get some of them to work one by one...

Hm, I wonder if this can work:

I didn't know namespaces existed in Neovim, but apparently they do: https://github.com/nvim-neotest/neotest/blob/26ed90509c377d10dbdebd25b7094a886323b32b/lua/neotest/types/init.lua#L3-L9

The args.tree:data().pos.type gives me the namespace name in the build_spec interface function, which is great!
That gives me the option to potentially execute test suites differently from directories (or actually, packages), files and individual tests.

The position id for the individual test becomes something like this, which is also great:

{
  id = "/Users/fredrik/code/public/neotest-golang/tests/go/testify_test.go::ExampleTestSuite::TestExample",
  name = "TestExample",
  path = "/Users/fredrik/code/public/neotest-golang/tests/go/testify_test.go",
  range = { 26, 0, 29, 1 },
  type = "test"
}

Right now, the test command won't come out proper and the test won't get executed, but that can be fixed.

Also, there's a regex (Test|Example|Suite) to match with the namespace/receiver, but that can be removed. Neotest will only pick up on the test under the receiver if the method starts with ^Test anyway, which might be ok.

I haven't fully wrapped my head around what drawbacks/caveats there are here...

EDIT: I had a bran fart. I will have to prepend Test to the namespace in the scenario above to make it work. This means I'm dictating how struct naming must be done...

EDIT 2: The neotest-python adapter "auguments" the AST-detected test names with runtime-collected data. It's possible this technique would be helpful here...

EDIT 3: A potential solution would be to create a lookup table, which gets created right after test discovery. The known (potential) suite would be ExampleTestSuite and custom AST detection would map it to TestExampleTestSuite. The lookup could look something like { "ExampleTestSuite" : "TestExampleTestSuite" }. This custom AST detection could potentially be done as part of discovering positions, and it could potentially augument the test position data with the actual namespace name. Either way, it sounds like testify suites will defintively be an opt-in feature - if we can make this work.

jstensland · 2024-06-26T14:35:52Z

You you're determining the name TestExampleTestSuite by convention of the example?

func TestExampleTestSuite(t *testing.T) {
    suite.Run(t, new(ExampleTestSuite))
}

Taking a quick look through a few repos, I see the struct is often private exampleTestSuite, which might lead to TestexampleTestSuite, if I'm reading this right. That would not match anything.

The potential to be in a different file is a tricky edge case as well with how neotest is set up. Go considers all files in a package in scope. Does this mean given the neotest interface the adapter would need to parse the whole package for each file to have chance at a more deterministic route? As in, it would have to do a few queries to trace down the right Name space and suite runner test?

Feel free to just point me at some docs. All I have found is the readme overview pointing to here, and I haven't had the chance to read through neotest.

Perhaps another adapter could be an inspiration too. What other supported languages compile this way... 🤔

fredrikaverpil · 2024-06-26T15:47:53Z

@jstensland I added some edits to my previous comment and I did not realize this until it was too late, but I got the namespace from the receiver (ExampleTestSuite), not from the suite test function. You can see in my screenshot by the name of the suite that it is incorrect. And thus, the actual test TestExampleTestSuite/TestExample is not found and won't execute because it took the receiver as namespace.

I think that supporting test suites will be super difficult just in general, because I cannot run a go test or go list command which will give me back a solid single source of truth in terms of test names, from go runtime. We'll always have to rely on some sort of AST detection and/or code inspection shenanigans. Or perhaps we could solve it by requiring a manual test run to populate runtime data into neotest-golang, but I am not a big fan of that and doubt the complexity is going to be ever worth it.

But if we limit the scope to at least initially only support testify, it should be possible to do support this using tree-sitter AST detection;

We could to look for suite.Run(t, new(xxx) and store xxx for later, as the key in a lookup table: { "xxx": nil }
We then need to fetch the function holding this t.Run. That function's name which would be our namespace. We'll place this function name in the lookup: { "xxx": "TestSuiteFunction" }. This lookup is required because the test function and the actual test might not be located in the same file.
Then to find candidates for potential suite tests, we need to detect tests defined on the receiver xxx, like func (suite *xxx) TestMe { ... }.
I'm not entirely sure what's the best path forward, but finally we'd like to have the Neotest tree show the actual namespace, not the receiver name. And we also want to correct test to be executed (again, not with the receiver as namespace). I haven't looked into mutating the position tree returned from the treesitter lib in the adapter's discover_positions function, but I think it can be done easily in the discover_positions function before returning. It'll slow the adapter down though because of this constant parsing, especially in a large code base - hence this having to be a opt-in configuration. It wouldn't be far-fetched to investigate if this can be offloaded to Go and if that would be faster...

Taking a quick look through a few repos, I see the struct is often private exampleTestSuite, which might lead to TestexampleTestSuite, if I'm reading this right. That would not match anything.

I don't quite follow. You mean like this?

func TestSuite(t *testing.T) {
    suite.Run(t, new(privateStruct))
}

The above should be totally fine.

Feel free to just point me at some docs.

Sorry, I don't think there are any. You have to install lazydev.nvim and dive into Neotest's codebase. You can take inspiration from neotest-python, neotest-zig, neotest-rust and other fairly recently developed adapters too.

But anyway, have a look at ast.lua https://github.com/fredrikaverpil/neotest-golang/pull/58/files#diff-2bea1cce72f6f14447e94556ded816a451e8362ee57ecf237d8212296d1cf2b9 and the newly added contribution info on tree-sitter/AST detection Neovim commands in this project's README.

You can parse code using any logic you want here. Could even drop to Go and run AST detection with Go to do certain things where the Neotest position tree structure isn't important, such as when creating a receiver/namespace lookup.

uroborosq · 2024-06-27T19:03:13Z

From my side, here are two main problems:

How to match suite method with ordinary test function (parent), which contains suite.Run(...)
How to translate to Neotest our tests structure.

For first one, we have two options:

Do several queries, use some map SuiteName (aka receiver) : test function. It must be easier way, and I think, that overhead of running a few queries won't be noticeable. Possibly, it will be simpler to use this method to solve the problem, when suite method and parent function are in different files.
Want to notice, that overriding build_position function in the opts for parse_positions function can be useful - https://github.com/nvim-neotest/neotest/blob/master/lua/neotest/lib/treesitter/init.lua#L95. In this function it's possible to get access to all query captures.
Try to do this with one query, as I tried to do. It is achievable to do this comparing receiver of test method == type parameter in suite.Run. My queries detects it (at least for my tests :) ). The drawback of this solution is that neotest.lib.treesitter.parse_positions applies to only one file. We may don't use it, but of course it's really not desirable. And also I haven't found out yet how to parse several files with treesitter query from lua api. Although, it looks like possible in general - http://tree-sitter.github.io/tree-sitter/creating-parsers#command-parse. And I don't know whether query can detect parent test function in one file and suite method in another.

So, generally first point looks doable. I think, that it might be OK to accept this limitation for different files for the initial support.

But second one looks harder. As I understand, Neotest tree design has very restricted structure:
2. Namespace or test must have one uninterrupted range (begin and end of node in the source code).

As I can see, dir contains files, files contains namespaces, namespaces contains tests and tests can also contain tests. For example, this line indirectly proves this - https://github.com/nvim-neotest/neotest/blob/master/lua/neotest/lib/positions/init.lua#L57. So file can't have any parent, but in our case namespace of whole suite should contain files. It is again about problem when parent test function and suite method are in different files.
Even for one single file I has a problem that namespace (or test) as neotest.Position has field range which is uninterrupted. As I can see, this range is used by neotest to determine the nearest test. But methods of suite can be mixed with other functions (and of course be in other files (again)).
I has such example:

func TestProductService(t *testing.T) {
	suite.Run(t, new(ProductServiceSuite))
}

type ProductServiceSuite struct {
	suite.Suite
}

func (s *ProductServiceSuite) TestKEK() {
	cases := []struct {
		name string
		err  error
	}{
		{
			name: "kek",
			err:  assert.AnError,
		},
		{
			name: "lol",
		},
	}

	for _, test := range cases {
		s.Run(test.name, func() {
			if test.err != nil {
				s.Fail(test.err.Error())
			}
		})
	}
}

func TestSmth(t *testing.T) {
	t.Run("noname", func(t *testing.T) {
		t.Fail()
	})

	t.Run("newname", func(t *testing.T) {
	})
}

func (s *ProductServiceSuite) TestLOL() {
	s.Run("first", func() {
		const a = 4
		if 1 != 2 || a == 4 {
			s.Fail("oh no")
		}
	})

	s.Run("kek", func() {
		if 2 == 3 {
			s.Fail("yes")
		}
	})
}

There is one suite with two methods, which separated with one standard test function.

With my queries, it produces the following neotest tree:

{ {
    id = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
    name = "service_test.go",
    path = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
    range = { 0, 0, 63, 0 },
    type = "file"
  }, { {
      id = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go::TestProductService",
      name = "TestProductService",
      path = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
      range = { 9, 0, 11, 1 },
      type = "test"
    } }, { {
      id = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go::TestProductService",
      name = "TestProductService",
      path = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
      range = { 17, 0, 38, 1 },
      type = "namespace"
    }, { {
        id = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go::TestProductService::TestKEK",
        name = "TestKEK",
        path = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
        range = { 17, 0, 38, 1 },
        type = "test"
      }, { {
          id = '/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go::TestProductService::TestKEK::"kek"',
          name = '"kek"',
          path = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
          range = { 22, 2, 25, 3 },
          type = "test"
        } }, { {
          id = '/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go::TestProductService::TestKEK::"lol"',
          name = '"lol"',
          path = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
          range = { 26, 2, 28, 3 },
          type = "test"
        } } } }, { {
      id = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go::TestSmth",
      name = "TestSmth",
      path = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
      range = { 40, 0, 47, 1 },
      type = "test"
    }, { {
        id = '/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go::TestSmth::"noname"',
        name = '"noname"',
        path = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
        range = { 41, 1, 43, 3 },
        type = "test"
      } }, { {
        id = '/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go::TestSmth::"newname"',
        name = '"newname"',
        path = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
        range = { 45, 1, 46, 3 },
        type = "test"
      } } }, { {
      id = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go::TestProductService",
      name = "TestProductService",
      path = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
      range = { 49, 0, 62, 1 },
      type = "namespace"
    }, { {
        id = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go::TestProductService::TestLOL",
        name = "TestLOL",
        path = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
        range = { 49, 0, 62, 1 },
        type = "test"
      }, { {
          id = '/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go::TestProductService::TestLOL::"first"',
          name = '"first"',
          path = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
          range = { 50, 1, 55, 3 },
          type = "test"
        } }, { {
          id = '/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go::TestProductService::TestLOL::"kek"',
          name = '"kek"',
          path = "/home/uroborosq/desktop/single_projects/shoplanner/go-backend/internal/product/service_test.go",
          range = { 57, 1, 61, 3 },
          type = "test"
        } } } } }

As you can see, there are two TestProductService namespaces, which are almost the same, except range. This produce the weird output, which I've posted later.
So it looks like some changes in neotest are required or some compromises decisions on the adapter side.

fredrikaverpil · 2024-06-27T23:05:13Z

@uroborosq I think there is a fairly simple solution to support multiple test files, by leveraging a receiver-testfunction lookup like I explained above/earlier. Do you see a caveat or drawback with this?

For the second problem, I would have to tinker a bit with postprocessing of the positions returned by the Neotest/treesitter lib currently used. Either we can postprocess the position nodetree directly or create a supporting data structure. The latter could be less coupled, less disruptive and less problematic with future changes to Neotest but could also be noticeably slower.

I am pretty confident I can solve this, but with the limitation of only supporting Testify (as this seems to follow a suite.Run pattern) and not custom-built suites. But I will very likely have to do custom AST-parsing, not leveraging the Neotest-provided lib. I would even consider doing the AST parsing in Go. In short, a considerable amount of dev time.

Having all this said, and as a side note, I would personally like to focus on getting some basic support in for running all tests in a file using one go test command, detect more table test syntax variations and fix some visual status related bugs before attempting to tackle this. But you are of course welcome to also take a stab at this any time. But it could be that some learning (on my end) as I develop aforementioned features will lead to increased understanding and the possibilities of having new avenues opened of seeing potential solutions.

Supporting test suites will be tricky and I want to carefully decide where to add the complexity required. Quite frankly, this increased insight/understanding while exploring this topic so far has given me a reason not to ever want to use test suites (not limited to testify) in my own projects.

I really wish go test or go list will one day be capable of give a reliable list of all tests (including namespaces and nested/table tests) by relying on runtime inference which would be super reliable.

jstensland · 2024-06-27T23:42:16Z

Thanks for the responses. I am away for a bit but will come back and look more deeply later next week, although you're miles ahead 🙂. A few thoughts in case they can be helpful.

It looks to me like testify uses the reflect package to find it's methods and build out test for testing.T. Accordingly, I think the go tools aren't ever going to help the way you want. Although, perhaps if it's an optional configuration anyways, could gopls close the gap?

Suite detection in VSCode go extension. Regex rather than tree sitter and I'm sure the internals of recording the tests are different, to the larger problems discussed. At least an example of cases they are trying to cover between the comment and regex

fredrikaverpil · 2024-07-05T00:20:29Z

A quick update on this topic. I just merged in the last stuff I wanted to make sure I got right for this adapter. It's basically at a mental v1.0.0 for me. 🎉

This means, unless there are bugs reported, testify suites (and suites in general) is the main enhancement I'll look into next. However, I also have a vacation coming up so I'm not going to make promises in terms of timeframes. 😄

fredrikaverpil · 2024-07-06T23:42:37Z

If you like, you can give this a go and see what weird behavior you might find 😄

{
    "fredrikaverpil/neotest-golang",
    branch = "feat/test-suite-support",
}

Loose thoughts:

Should probably AST-parse all files in the same package (or broader, as structs can reside in other package) when creating the lookup. Then I could replace the node ids in the tree (returned by ast.lua) instead of injecting hacks all over the place.
Debugging of a test doesn't work right now, but could be made functional by replacing the pos.ids directly in the tree.

fredrikaverpil · 2024-07-07T12:47:38Z

Some further progress...

fredrikaverpil · 2024-07-07T15:12:49Z

Merged namespaces ✅
Debugging ✅
Opt-in with option testify = true

  {
    "nvim-neotest/neotest",
    ft = { "go" },
    dependencies = {
      {
        "fredrikaverpil/neotest-golang",
        branch = "feat/test-suite-support",
      },
    },
    opts = function(_, opts)
      opts.adapters = opts.adapters or {}
      opts.adapters["neotest-golang"] = {
        dap_go_enabled = true,
        testify = true,  -- experimental
      }
    end,
  },

fredrikaverpil · 2024-07-07T15:14:42Z

Would be nice if you had the time to test drive this some in actual projects, and provide feedback!
I'm off to the 🏖️ 😎

jstensland · 2024-07-07T19:04:02Z

Tried testing with branch = "feat/test-suite-support", but running into the following on first invocation of run (shown below), summary, etc.

   Error  11:15:04 msg_show.emsg E5108: Error executing lua: ...im/lazy/LazyVim/lua/lazyvim/plugins/extras/test/core.lua:111: attempt to index field 'run' (a nil value)
stack traceback:
	...im/lazy/LazyVim/lua/lazyvim/plugins/extras/test/core.lua:111: in function <...im/lazy/LazyVim/lua/lazyvim/plugins/extras/test/core.lua:111>
   Error  11:15:04 notify.error lazy.nvim Failed to run `config` for neotest

.../nvim/lazy/neotest-golang/lua/neotest-golang/testify.lua:239: E565: Not allowed to change text or change window

# stacktrace:
  - /neotest-golang/lua/neotest-golang/testify.lua:239 _in_ **run_query_on_file**
  - /neotest-golang/lua/neotest-golang/testify.lua:177 _in_ **generate_lookup_map**
  - /neotest-golang/lua/neotest-golang/init.lua:201 _in_ **adapter**
  - /LazyVim/lua/lazyvim/plugins/extras/test/core.lua:93 _in_ **config**

fredrikaverpil · 2024-07-07T22:14:23Z

So it looks like it choked on this line for you?

  -- neotest-golang/lua/neotest-golang/testify.lua:239
  vim.api.nvim_buf_set_lines(bufnr, 0, -1, false, content)

Hm, strange. I wonder why I'm not also getting that. I made basically the final touches on the overall functionality in the PR now, commit 7c720b4. If you pull the latest commit in the PR, do you still see this error?

You're on LazyVim and on Neovim 0.10.0 stable?

fredrikaverpil · 2024-07-07T22:24:41Z

@nikosbatsaras @evgeniy-krivenko @thelooter @uroborosq @roveo @zankich sorry for the spam here, but if you think it's worth your time, please help me and have a look at #58 (in terms of trying it out) where I've implemented stretchr/testify support. Would like to gather problems, such as the one highlighted in the post just above this one, so to understand what works and what doesn't work before I start refactoring, cleanup and add some tests.

fredrikaverpil · 2024-07-08T05:55:44Z

@jstensland I was able to reproduce your problem in LazyVim and I believe I pushed a valid fix for it. Have another look, please! 😄

jstensland · 2024-07-08T16:47:22Z

Issue is resolved 🎉

Poking around, things are working alright, but I do have one suite that's getting skipped and I don't see why. It's using the new(suiteName) construction. It's am mix of the suite and normal go tests in the file, but I tested that works elsewhere. I'll try to look at it more later to provide some more helpful information.

fredrikaverpil · 2024-07-08T18:28:45Z

@jstensland I'm pretty sure you are hitting the same issue as @nikosbatsaras is hitting here: #58 (comment)

Can you post what your test suite function looks like?

fredrikaverpil · 2024-07-08T18:30:49Z

Oh wait, you say you are already using that new(yourStruct) declaration.

So, another issue right now is that I need to figure out how to trigger a rebuild of the lookup as you edit your code. It gets created only once right now, at adapter initialization. Could that be it?

If you can provide a minimal example, I can dig into it.

jstensland · 2024-07-08T21:16:34Z

Most suites worked, and my example that doesn't work is in a private repo. I tried to recreate it and don't reproduce it. I'll see what I can do.

~~It does look like its running the right go test command, so detection needed for that is working.~~

jstensland · 2024-07-08T22:55:52Z

I wasn't reading the run spec close enough I guess. It doesn't produce the right suite namespace.

It looks like the failing cases have another test after the suite in the file.

To reproduce, put the following test at the bottom of tests/go/testify1_test.go.

func TestTrivial(t *testing.T) {
	assert.Equal(t, 5, 5)
}

The go up to TestExample1 and run the nearest. It tries to run with -run ^TestTrivial/TestExample1$ instead of -run ^TestSuite/TestExample1$

fredrikaverpil · 2024-07-09T04:13:39Z

Very good find @jstensland - please try the latest commit in the branch. I pushed a fix for this:

jstensland · 2024-07-09T16:59:00Z

tested where I originally had the issues and looks fixed! thanks!

fredrikaverpil · 2024-07-09T17:22:05Z

Thanks for verifying @jstensland. So, the essential functionality is in place and I'm thinking I'll try and get what we have now merged but with some refactoring, polish and tests. So in the meantime, feel free to run neotest-golang with the feat/test-suite-support branch.

jstensland · 2024-07-11T17:43:06Z

I've been using this and it's quite nice. Thank you for all the work @fredrikaverpil

fredrikaverpil · 2024-07-11T17:47:53Z

Thanks for your feedback @jstensland ❤️
I'm actually about to merge this into main now. Please note that the option to enable the feature was renamed to testify_enabled.

jstensland · 2024-07-11T17:48:52Z

I just updated and caught that. thanks 👍

fredrikaverpil changed the title ~~Support running single test in a suite~~ Support running single test in a testify suite Jun 6, 2024

fredrikaverpil added the enhancement New feature or request label Jun 11, 2024

fredrikaverpil mentioned this issue Jun 25, 2024

feat: testify test suite support #58

Merged

28 tasks

This comment was marked as outdated.

Sign in to view

fredrikaverpil closed this as completed in #58 Jul 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support running single test in a testify suite #25

Support running single test in a testify suite #25

jstensland commented Jun 5, 2024

fredrikaverpil commented Jun 5, 2024 •

edited

Loading

thelooter commented Jun 5, 2024

jstensland commented Jun 5, 2024

fredrikaverpil commented Jun 6, 2024

fredrikaverpil commented Jun 6, 2024 •

edited

Loading

jstensland commented Jun 6, 2024

uroborosq commented Jun 19, 2024

fredrikaverpil commented Jun 25, 2024 •

edited

Loading

jstensland commented Jun 26, 2024

fredrikaverpil commented Jun 26, 2024

uroborosq commented Jun 27, 2024

fredrikaverpil commented Jun 27, 2024

jstensland commented Jun 27, 2024

fredrikaverpil commented Jul 5, 2024

This comment was marked as outdated.

fredrikaverpil commented Jul 6, 2024 •

edited

Loading

fredrikaverpil commented Jul 7, 2024

fredrikaverpil commented Jul 7, 2024 •

edited

Loading

fredrikaverpil commented Jul 7, 2024

jstensland commented Jul 7, 2024

fredrikaverpil commented Jul 7, 2024 •

edited

Loading

fredrikaverpil commented Jul 7, 2024 •

edited

Loading

fredrikaverpil commented Jul 8, 2024

jstensland commented Jul 8, 2024

fredrikaverpil commented Jul 8, 2024

fredrikaverpil commented Jul 8, 2024 •

edited

Loading

jstensland commented Jul 8, 2024 •

edited

Loading

jstensland commented Jul 8, 2024

fredrikaverpil commented Jul 9, 2024

jstensland commented Jul 9, 2024

fredrikaverpil commented Jul 9, 2024

jstensland commented Jul 11, 2024

fredrikaverpil commented Jul 11, 2024

jstensland commented Jul 11, 2024

Support running single test in a testify suite #25

Support running single test in a testify suite #25

Comments

jstensland commented Jun 5, 2024

fredrikaverpil commented Jun 5, 2024 • edited Loading

thelooter commented Jun 5, 2024

jstensland commented Jun 5, 2024

fredrikaverpil commented Jun 6, 2024

fredrikaverpil commented Jun 6, 2024 • edited Loading

jstensland commented Jun 6, 2024

uroborosq commented Jun 19, 2024

fredrikaverpil commented Jun 25, 2024 • edited Loading

jstensland commented Jun 26, 2024

fredrikaverpil commented Jun 26, 2024

uroborosq commented Jun 27, 2024

fredrikaverpil commented Jun 27, 2024

jstensland commented Jun 27, 2024

fredrikaverpil commented Jul 5, 2024

This comment was marked as outdated.

fredrikaverpil commented Jul 6, 2024 • edited Loading

fredrikaverpil commented Jul 7, 2024

fredrikaverpil commented Jul 7, 2024 • edited Loading

fredrikaverpil commented Jul 7, 2024

jstensland commented Jul 7, 2024

fredrikaverpil commented Jul 7, 2024 • edited Loading

fredrikaverpil commented Jul 7, 2024 • edited Loading

fredrikaverpil commented Jul 8, 2024

jstensland commented Jul 8, 2024

fredrikaverpil commented Jul 8, 2024

fredrikaverpil commented Jul 8, 2024 • edited Loading

jstensland commented Jul 8, 2024 • edited Loading

jstensland commented Jul 8, 2024

fredrikaverpil commented Jul 9, 2024

jstensland commented Jul 9, 2024

fredrikaverpil commented Jul 9, 2024

jstensland commented Jul 11, 2024

fredrikaverpil commented Jul 11, 2024

jstensland commented Jul 11, 2024

fredrikaverpil commented Jun 5, 2024 •

edited

Loading

fredrikaverpil commented Jun 6, 2024 •

edited

Loading

fredrikaverpil commented Jun 25, 2024 •

edited

Loading

fredrikaverpil commented Jul 6, 2024 •

edited

Loading

fredrikaverpil commented Jul 7, 2024 •

edited

Loading

fredrikaverpil commented Jul 7, 2024 •

edited

Loading

fredrikaverpil commented Jul 7, 2024 •

edited

Loading

fredrikaverpil commented Jul 8, 2024 •

edited

Loading

jstensland commented Jul 8, 2024 •

edited

Loading