olimorris / neotest-rspec

๐Ÿงช Neotest adapter for RSpec. Works in Docker containers too
MIT License
88 stars 25 forks source link

Wrong highlight on the line that was not the one that failed the test #55

Closed samnang closed 1 year ago

samnang commented 1 year ago

Every time there is a wrong expectation in the test, it will highlight on a wrong line. From what I noticed, it always highlights on the first line of the test/example. The correct behavior should highlight the line that caused the test to fail with the wrong expectation.

_usr_local_bin_nvim
olimorris commented 1 year ago

Can you try and recreate with this minimal config:

local root = vim.fn.fnamemodify("./.repro", ":p")

-- set stdpaths to use .repro
for _, name in ipairs({ "config", "data", "state", "cache" }) do
  vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
end

-- bootstrap lazy
local lazypath = root .. "/plugins/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
  vim.fn.system({
    "git",
    "clone",
    "--filter=blob:none",
    "--single-branch",
    "https://github.com/folke/lazy.nvim.git",
    lazypath,
  })
end
vim.opt.runtimepath:prepend(lazypath)

-- install plugins
local plugins = {
  {
    "nvim-neotest/neotest",
    lazy = true,
    dependencies = {
      "olimorris/neotest-rspec",
    },
    config = function()
      require("neotest").setup({
        adapters = {
          require("neotest-rspec"),
        },
      })
    end,
  },
}

require("lazy").setup(plugins, {
  root = root .. "/plugins",
})

If you run nvim --clean -u minimal.lua to load the minimal config then test again. I can then work on a bugfix

samnang commented 1 year ago

@olimorris as your suggestion, I tried to start the minimal.lua in my project root, but it said "No tests found" when I tried to run on test file with this command lua require("neotest").run.run().

local root = vim.fn.fnamemodify("./.repro", ":p")

-- set stdpaths to use .repro
for _, name in ipairs({ "config", "data", "state", "cache" }) do
    vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
end

-- bootstrap lazy
local lazypath = root .. "/plugins/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
    vim.fn.system({
        "git",
        "clone",
        "--filter=blob:none",
        "--single-branch",
        "https://github.com/folke/lazy.nvim.git",
        lazypath,
    })
end
vim.opt.runtimepath:prepend(lazypath)

-- install plugins
local plugins = {
    {
        "nvim-neotest/neotest",
        lazy = true,
        dependencies = {
            "nvim-lua/plenary.nvim",
            "nvim-treesitter/nvim-treesitter",
            "antoinemadec/FixCursorHold.nvim",
            "olimorris/neotest-rspec",
        },
        config = function()
            require("neotest").setup({
                adapters = {
                    require("neotest-rspec"),
                },
            })
        end,
    },
}

require("lazy").setup(plugins, {
    root = root .. "/plugins",
})
olimorris commented 1 year ago

Arghhh. Of course it did. I forgot to include Treesitter:

local root = vim.fn.fnamemodify("./.repro", ":p")

-- set stdpaths to use .repro
for _, name in ipairs({ "config", "data", "state", "cache" }) do
  vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
end

-- bootstrap lazy
local lazypath = root .. "/plugins/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
  vim.fn.system({
    "git",
    "clone",
    "--filter=blob:none",
    "--single-branch",
    "https://github.com/folke/lazy.nvim.git",
    lazypath,
  })
end
vim.opt.runtimepath:prepend(lazypath)

-- install plugins
local plugins = {
  {
    "nvim-treesitter/nvim-treesitter", -- Smarter code understanding like syntax Highlight and navigation
    build = ":TSUpdate",
    config = function()
      require("nvim-treesitter.configs").setup({
        ensure_installed = "all",
      })
    end,
  },
  {
    "nvim-neotest/neotest",
    lazy = true,
    dependencies = {
      "olimorris/neotest-rspec",
    },
    config = function()
      require("neotest").setup({
        adapters = {
          require("neotest-rspec"),
        },
      })
    end,
  },
}

require("lazy").setup(plugins, {
  root = root .. "/plugins",
})

Apologies...this is what happens when you don't set an issue template in GitHub.

Don't forget to rm -rf the .repro folder too.

samnang commented 1 year ago

Ah, I forget to install the treesitter. After I installed the treesitter and started with nvim --clean -u minimal.lua, I still got the same result which poiting to the wrong line.

image
olimorris commented 1 year ago

This is good thank you. Can you share your full minimal file, including Neotest config, and I can take a look?

olimorris commented 1 year ago

Actually, I've been able to recreate this. It's because the command we're sending to RSpec is for the "it" block, and not for the expect block.

That being said, the result should be shown on the line above and I've fixed that in 1b6ad445236bbe453bf9217aa7f82a5afff33f15.

I'll see what we can do about putting the failure on the specific line...

samnang commented 1 year ago

Thank @olimorris, now it shows as virtual text at the same line as it block.

I'll see what we can do about putting the failure on the specific line...

For putting the failure on the specific line, I notice neotest-jest adapter has the correct behaviour that we expected.

olimorris commented 1 year ago

Appreciate the tip! I haven't looked at RSpec for a while so it might be just a feature of jest but I will take a look.

olimorris commented 1 year ago

So digging into how RSpec formats JSON output (which we use in the adapter to parse and feed back to Neotest), I can see that there isn't anything in there which outputs the actual line number responsible for the failed assertion.

I even had a look at the failures formatter but that is somewhat lacking too.

I'll do some more digging into this but it may be a PR to rspec-core or the creating of a custom formatter.

samnang commented 1 year ago

It's interesting to know there is no failed line number property directly in JsonFormatter, but I notice there are backtrace and file_path properties in there, is it possible to match and get the first one because it's already in reverse order? Then we can extract the line number from there.

image
olimorris commented 1 year ago

Brilliant spot!

In the latest commit this should be fixed. Let me know ๐Ÿ‘๐Ÿผ

samnang commented 1 year ago

Thank @olimorris, it works as expected ๐Ÿ‘