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

fix unit test running #65

Merged
merged 3 commits into from
Nov 29, 2023
Merged

Conversation

jstensland
Copy link
Contributor

Changes

  • Specify how to run the unit tests
  • Run neotest setup in the tests so the extra args match the test expectations
  • remove reference to sgetman user

Notes

I'm new to lua. Is there's a better way to get get plenary and neotest in the workspace? I don't love relying on them being adjacent to this repo. I added them as the LSP couldn't find the neotests types like ---@type neotest.Adapter, or plenary functions like it

Copy link
Collaborator

@sergii4 sergii4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, @jstensland! Thanks for contributing. All looks good, just left a minor comment

adapters = {
require("neotest-go")({
experimental = {
test_table = true,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we make it default on the future to not specify directly

@@ -33,6 +33,17 @@ assert:register(
"assertion.has_property.negative"
)

require("neotest").setup({
Copy link
Collaborator

@sergii4 sergii4 Nov 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the motivation for adding it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests were failing for me because I have different args set in my own config.

This seemed to fix the test expectations but also didn't overwrite them after I had run tests.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it makes sense to put it to master if it's specific for you? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not specific to me. It's specific to the test expectations. With no args specified in my own config, multiple tests fail. including

Fail	||	build_spec build specification for many_table_test.go
            ...re/nvim/site/pack/packer/start/neotest/lua/nio/tests.lua:27: Test task failed with message:
            The coroutine failed with this message:
            ...ensla/repos/neotest-go/lua/spec/neotest-go/init_spec.lua:358: Expected objects to be the same.
            Passed in:
            (string) 'cd /Users/jstensla/repos/neotest-go/neotest_go && go test -v -json  ./...'
            Expected:
            (string) 'cd /Users/jstensla/repos/neotest-go/neotest_go && go test -v -json  -count=1 -timeout=60s ./...'
            stack traceback:
            	[C]: in function 'error'
            	...e/pack/packer/start/plenary.nvim/lua/luassert/assert.lua:51: in function 'same'
            	...ensla/repos/neotest-go/lua/spec/neotest-go/init_spec.lua:358: in function <...ensla/repos/neotest-go/lua/spec/neotest-go/init_spec.lua:349>

            stack traceback:
            	...re/nvim/site/pack/packer/start/neotest/lua/nio/tests.lua:27: in function <...re/nvim/site/pack/packer/start/neotest/lua/nio/tests.lua:12>

What I meant was that they also fail if I specify whatever I want for my configuration (I setup a coverage arg), but that's (somewhat) beside the point. I am guessing you have these args set for yourself if the tests pass without this setup in the test

Copy link
Collaborator

@sergii4 sergii4 Nov 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a doubts because we have already defined plugin on L3

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I am open to suggestions on the best setup/teardown approach with Plenary's busted tests. This is my first time using them.

I switched to using the loaded plugin for setup

.luarc.json Outdated
"workspace.checkThirdParty": false,
"workspace.library": [
"../neotest",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running nvim --headless -c ':PlenaryBustedDirectory lua/spec' works for me without direct reference to those libraries. My understanding was that those dependencies are satisfied through plugin management.

Copy link
Collaborator

@sergii4 sergii4 Nov 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was running this on new Ubuntu machine with a minimal config from here and it worked without workspace.library. I believe we can remove that piece

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hapoy to remove if you like.

To clarify, it isn't needed for running the tests for me either, as those are loaded in my vim plugin. They were needed to make the LSP aware of those plugins so that it recognized the type annotations that reference neotest types, and then the plenary/busted functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@@ -192,52 +203,52 @@ describe("discover_positions", function()
local positions = plugin.discover_positions(path):to_list()
local expected_positions = {
{
id = "/Users/sgetman/.local/share/nvim/site/pack/packer/start/neotest-go/neotest_go/map_table_test.go",
id = vim.loop.cwd() .. "/neotest_go/map_table_test.go",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for correcting paths! 👍

Copy link
Collaborator

@sergii4 sergii4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@sergii4 sergii4 merged commit c8ad8cc into nvim-neotest:main Nov 29, 2023
1 check passed
@jstensland jstensland deleted the fix/unit-test-and-lsp branch November 30, 2023 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants