olimorris / neotest-rspec

🧪 Neotest adapter for RSpec. Works in Docker containers too
MIT License
88 stars 25 forks source link

Create custom RSpec reporter #17

Closed olimorris closed 2 years ago

olimorris commented 2 years ago

I feel we're reaching the limit of the RSpec JSON formatter. It's becoming harder and harder to connect the output from Treesitter with RSpec. I've started to look into a custom RSpec reporter to begin to see what we could use

compactcode commented 2 years ago

Curious what you were thinking. Here is my initial idea:

The tree sitter queries provide a range which appears to match up to the line number of the test:

Treesitter ID: {
  id = "Sum when passing test #call should return the sum of two numbers",
  name = "'should return the sum of two numbers'",
  path = "/home/shandogs/code/public/neotest-rspec/spec/simple_example_spec.rb",
  range = { 10, 6, 12, 9 },
  type = "test"
}

It seems that 10, 6 is the line, column of the start of the test (assuming the ranges are 0 indexed).

1655809751

When we run the spec we get a file and line number in the example output:

https://github.com/olimorris/neotest-rspec/blob/4d558da22ca64ae2dd3f2ec5c68eab629be5a569/tests/utils_spec.lua#L22-L23

Could we tie these two things together using the spec and tree arguments passed from neotest?

function NeotestAdapter.results(spec, result, tree)
olimorris commented 2 years ago

I think we found that at the same time 😄 .

So I may have cracked it...just ran the tests you added today and I'm getting green. Fix is on the rspec-formatter branch so would love your feedback and testing.

You'll see a ./lib/neovim_formatter.rb file which I've hacked from the RSpec JSON formatter. From this I've formed a treesitter_id in the rspec output.

olimorris commented 2 years ago

EDIT: I just need to amend the path to the neovim_formatter.rb in the command!

olimorris commented 2 years ago

@compactcode ...some big changes ahoy. I've made a custom gem which handles all of the RSpec formatting. It felt neccessary to abstract the formatting away from this plugin and remove the ./lib/neovim_formatter.rb file but it is now a dependency in your Gemfile.

As always, would love your feedback!

I think I've broken our tests so will fix later.

compactcode commented 2 years ago

I'm curious why we need the custom formatter. Could we do the equivalent of:

treesitter_id: "#{test.metadata[:absolute_file_path]}::#{test.metadata[:line_number] - 1}",

Inside M.parse_json_output with the existing formatter?

Regarding the gem, do you mean we would need to add the gem to every project we want to use neotest-rspec on? If so that is probably a deal breaker for me, particularly on larger commercial projects where I might be the only one using it.

olimorris commented 2 years ago

Ahhhhh okay. I'll keep hacking with the file paths in Lua. Basically test.metadata[:absolute_file_path] doesn't exist in RSpec by default so the Gem unlocks that and then minimises the amount of file path manipulation we have to do in the adapter. I'll keep hacking

EDIT: I think vim.loop.getcwd() (or similar) will get us the path to add on to RSpec's file path

compactcode commented 2 years ago

Really appreciate this work and taking on the feedback, thanks. I think this new approach will fix a few other issues I had been having so looking forward to it.

Let me know if I can help with anything.

olimorris commented 2 years ago

I appreciate the dialogue and feedback. Great teamwork!

I've made a push to the branch which removes the dependency on the gem. I'm hopeful this resolves our issues.

compactcode commented 2 years ago

@olimorris had a quick play with the branch this morning and it is working great. It is now handling all the weird edge cases I was having with the old approach :tada:

Will continue using through the day and let you know if I spot any issues.

olimorris commented 2 years ago

Epic news! Wish we'd have thought of this approach sooner 🤣. Will merge into main this morning. Thanks for testing and your feedback.

Next step will be to add the dap strategy.