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

[mini.test] Ability to ignore screenshot attributes #1300

Open
2 tasks done
stasjok opened this issue Oct 21, 2024 · 5 comments
Open
2 tasks done

[mini.test] Ability to ignore screenshot attributes #1300

stasjok opened this issue Oct 21, 2024 · 5 comments
Labels
feature-request Request for a feature to existing module mini.test

Comments

@stasjok
Copy link
Contributor

stasjok commented Oct 21, 2024

Contributing guidelines

Module(s)

mini.test

Description

Screenshots give the ability to compare what is displayed on the screen and how it is displayed. But sometimes I only care of what is displayed, not how it's displayed. The latter can change from the external changes like colorscheme or something. I think in some cases an additional option to ignore screenshot attributes can be useful.

@stasjok stasjok added the feature-request Request for a feature to existing module label Oct 21, 2024
@echasnovski
Copy link
Owner

Thanks for the suggestion!

The use case seems more or less valid, but an actual implementation would require a bit messy source code changes. Mostly because for completeness it would also require to allow users to ignore text in favor of comparing only attributes, which is not ideal for my taste.

In general, I think that validating screenshots in full (both text and attributes) is a good practice for a reliable test. Here is what you can do to make comparing only text less painful:

  • Manually set custom color scheme. The one over which you have full control. This is what is done in 'mini.nvim'.
  • If troubles come from only a certain known lines (which don't contain tested text), then you can use opts.ignore_lines. This is commonly useful to ignore command line text/attributes or lines that contain text varying on testing instance (like full paths).
  • Resort to testing buffer text explicitly via comparing child.api.nvim_buf_get_lines(0, 0, -1, false) to a reference array.

Closing as not planned.

@ibhagwan
Copy link

ibhagwan commented Jan 26, 2025

Hi @echasnovski,

First let me say ty for the absolute awesomeness of mini.test, once I decided to up my ci game there was really no competition, mini.test is a gem and one of my favorite mini modules now (hard competition, I know 😄).

Just wanted to weigh in on this in the hopes you’ll reconsider. Similar to @stasjok I also needed to ignore the attrs of screenshots as these can be different between the stable/nightly and sometimes even Mac.

Since I didn’t want to “lose” certain screenshot testing on nightly and Windows I implemented my own child.get_screenshot and borrowed some code from the amazing @MagicDuck grug-far testing:

source

function M.fromChildScreen(child)
  local lines = child.lua([[
      local lines = {}
      for i = 1, vim.o.lines do
        local line_text = {}
        for j = 1, vim.o.columns do
          table.insert(line_text, vim.fn.screenstring(i, j))
        end
        table.insert(lines, table.concat(line_text))
      end
      return lines
    ]])
  return M.from_lines(lines)

Which is then called by a helper function (adapted from your code):

source

  child.get_screen_lines = function()
    return screenshot.fromChildScreen(child)
  end

  -- Expect screenshot without the "attrs" (highlights)
  child.expect_screen_lines = function(opts, path)
    opts = opts or {}
    opts.force = not not vim.env["update_screenshots"]
    if opts.redraw ~= false then child.cmd("redraw") end
    MiniTest.expect.reference_screenshot(child.get_screen_lines(), path, opts)
  end

Which I then use with a “parameterized” test set (with and without attrs):

source

    if attrs then
      child.expect_screenshot({ ignore_lines = ignore_lines })
    else
      child.expect_screen_lines({ ignore_lines = ignore_lines })
    end

The use case seems more or less valid, but an actual implementation would require a bit messy source code changes. Mostly because for completeness it would also require to allow users to ignore text in favor of comparing only attributes, which is not ideal for my taste.

While I don’t mind the “hacks” I think it would be nice to have a ignore_attrs option, I don’t think ignore_text is really required here but even if you wanna do it for completeness shouldn’t be hard to set the text to an empty string as is done with the attrs (when ignored).

In the hopes you’d reconsider this and ty for your amazing work!

@echasnovski
Copy link
Owner

Thanks for adopting 'mini.test' usage!

Did you try setting custom color scheme or is it the case of when Nightly has some different attributes in Command-line area? In the latter cases I usually resort to only do screenshot testing on Nightly+ (or sometimes only on stable release) which is also a bit more future proof.

In the hindsight, the ignore_lines should have been separate ingore_text_lines and ignore_attr_lines, but its original use case (ignoring screenshot parts which contain full path that depends on the test runner) really needs setting both.

As an appreciation for using 'mini.test' in such a vital plugin, let's reopen this so that I'll think about it more after next 'mini.nvim' stable release.

@echasnovski echasnovski reopened this Jan 26, 2025
@ibhagwan
Copy link

Did you try setting custom color scheme or is it the case of when Nightly has some different attributes in Command-line area? In the latter cases I usually resort to only do screenshot testing on Nightly+ (or sometimes only on stable release) which is also a bit more future proof.

I saw the suggestion but didn’t try that yet, I felt avoiding the attrs was more prudent in that specific test case (basic ui layout and command result), there are other places I do need the attr check on all platforms/versions and plan to do so for these (I just started to add ui screenshot tests).

In the hindsight, the ignore_lines should have been separate ingore_text_lines and ignore_attr_lines, but its original use case (ignoring screenshot parts which contain full path that depends on the test runner) really needs setting both.

Correct, paths are more problematic on screenshots due to the different dir structures of the CI env, also helps me to test windows using dir separator without forcing path normalization to use backslashes.

As an appreciation for using 'mini.test' in such a vital plugin, let's reopen this so that I'll think about it more after next 'mini.nvim' stable release.

Ty @echasnovski! Absolutely no rush on this as my workaround code works great (ty @MagicDuck).

@ibhagwan
Copy link

A few other things I forgot to mention:

  • The current workaround sets the attr lines to empty lines resulting in a screenshot file half empty at the bottom, it would be nice to have the screenshot file reduce to just the visible lines, will also make the expect message nicer to distinguish between reference and observed.

While reading TESTING.md I came across:

Sometimes hanging process will occur: it stops executing without any output. Most of the time it is because Neovim process is "blocked", i.e. it waits for user input and won't return from other call. Common causes are active hit-enter-prompt (solution: increase prompt height to a bigger value) or Operator-pending mode (solution: exit it). To mitigate this experience, most helper methods will throw an error if they can deduce that immediate execution will lead to hanging state.

Perhaps it’s query mentioning that this can be also overcome by using the new :help mopt in the nightly:

'messagesopt' 'mopt'	string	(default "hit-enter,history:500")
			global
	Option settings for outputting messages.  It can consist of the
	following items.  Items must be separated by a comma.

	hit-enter	Use a |hit-enter| prompt when the message is longer than
			'cmdheight' size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for a feature to existing module mini.test
Projects
None yet
Development

No branches or pull requests

3 participants