Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: support table tests for test case structs with no fields #155

Closed
2 tasks done
jstensland opened this issue Jul 29, 2024 · 10 comments · Fixed by #156
Closed
2 tasks done

feature: support table tests for test case structs with no fields #155

jstensland opened this issue Jul 29, 2024 · 10 comments · Fixed by #156
Labels
enhancement New feature or request

Comments

@jstensland
Copy link

Did you check docs and existing issues?

Is your feature request related to a problem? Please describe.

I have multiple packages with test cases where there's an anonymous struct of test cases and the cases do not include fields. The current support for table tests do not detect these. e.g.

func TestTableTestInlineStructLoopNotKeyed(t *testing.T) {
	for _, tc := range []struct {
		name     string
		x        int
		y        int
		expected int
	}{
		{"TableTest1", 1, 2, 3}, // no `name:` to match on
		{"TableTest2", 3, 4, 7},
	} {
		t.Run(tc.name, func(t *testing.T) {
			if Add(tc.x, tc.y) != tc.expected {
				t.Fail()
			}
		})
	}
}

Given the support for this functionality when the field is there, it would be nice to support this form as well.

Describe the solution you'd like to see.

I would like to be able to run the nearest test case when my cursor is on it, or to run a single test case from the summary

Describe alternatives you've considered.

The obvious alternative is to just run the full test, which does work.

Additional context

I tried updated the treesitter query for these. I could only get as far as matchings the struct def with the first argument to run in this form (similar to what exists) but wasn't sure if it's possible to ask treesitter for the index of the sibling in the list of declared field, such that I could identify that name field to capture the test name. It may be good enough to assume its first, given this is just matching a specific code pattern anyways

@jstensland jstensland added the enhancement New feature or request label Jul 29, 2024
@fredrikaverpil
Copy link
Owner

We should be able to support this by extending the current treesitter queries: https://github.com/fredrikaverpil/neotest-golang/blob/main/lua/neotest-golang/query.lua

If you want to take a stab at it, there are Go tests in positions_test.go (in which you would add the test you provided above) as well as a lua test file that asserts that the tests are detected by the neotest-golang adapter in positions_spec.lua. There are instructions in the README on how to play around with the treesitter queries in query.lua as well as how to run the lua tests with the Makefile.

@fredrikaverpil
Copy link
Owner

fredrikaverpil commented Jul 29, 2024

Sorry, I somehow missed you already looked into this. 😅

I could only get as far as matchings the struct def with the first argument to run in this form (similar to what exists) but wasn't sure if it's possible to ask treesitter for the index of the sibling in the list of declared field, such that I could identify that name field to capture the test name.

I see what you mean. Not sure myself without digging (not at my machine until next week).

@jstensland
Copy link
Author

Seems possible to just match the first string defined in the struct which is likely the right one. Seems like an option if there's not a better way to index the fields between struct definition and literal definitions

@fredrikaverpil
Copy link
Owner

fredrikaverpil commented Aug 1, 2024

Please give #156 a try?

Load the plugin by setting branch = "table-test-without-struct-fields" in the lazy plugin specs.

Seems possible to just match the first string defined in the struct which is likely the right one.

This is exactly what I did now.

@jstensland
Copy link
Author

Sorry for the long delay. Two things

  1. To match other test case detection, how about able also supporting the variation where the struct is defined outside of the for loop? Those are not currently detected. Adjusting my original example, that's something like
func TestTableTestInlineStructLoopNotKeyed(t *testing.T) {
	 testcases := []struct {
		name     string
		x        int
		y        int
		expected int
	}{
		{"TableTest1", 1, 2, 3}, // no `name:` to match on
		{"TableTest2", 3, 4, 7},
	} 
	for _, tc := testcases {
		t.Run(tc.name, func(t *testing.T) {
			if Add(tc.x, tc.y) != tc.expected {
				t.Fail()
			}
		})
	}
}
  1. I am getting the following error running one of the tests cases
    Test(s) not associated (not found/executed):  { '/path/to/file_test.go::TestName::"casename"::"secondstringfield"' }
    

@fredrikaverpil
Copy link
Owner

Sorry for the silence here. I'm quite busy at work and with life at the moment. I'll have a look when I have the time. Feel free to take a stab at it in case you wish.

@fredrikaverpil
Copy link
Owner

@jstensland I updated the query in #156
Please give this a try and let me know how this works for you:

{
  "fredrikaverpil/neotest-golang",
  branch = "table-test-without-struct-fields",
}

@kvalv
Copy link

kvalv commented Dec 4, 2024

Hi @fredrikaverpil I stumbled upon the same issue by coincidence and checked out #156. The fix seems to work great on my end! 😄

Only slightly surprising behaviour I found is that if you run the nearest test when the cursor is below the array of test cases (e.g. on the for _, tc := range cases { -- the last test will be run ("2 * (5 + 10)") and not all tests which is something I'd expect. Nothing critical, though!

Cursor just below test cases: runs only the last
image

Cursor above all test cases: runs all
image

@fredrikaverpil
Copy link
Owner

Hi @kvalv !

I understand what you mean, but this is actually expected/intended behavior. Neotest gives the adapter the ability to provide treesitter queries so to detect what is considered a test or test case (in the test table). You can see the t.Run(...) as a "test body" and is in itself not considered a test on its own. You can see where the green checkmark symbols to appear to understand which one is nearest to where your cursor is at.

I also think (I'm not entirely sure as I'm on the go right now) that Neotest looks upwards the file when it tries to find the nearest test, so to make things more intuitive.

But on a slightly different note, this PR has been open for a long time. Since you have verified that it works for you, I'm thinking we can merge this in. WDYT?

@kvalv
Copy link

kvalv commented Dec 6, 2024

Thanks for the clarification! Makes sense that neotest looks upward when finding the nearest test, so this is intended behaviour 👍

Since you have verified that it works for you, I'm thinking we can merge this in. WDYT?

Sounds great! From what I can see, the PR should resolve this issue 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants