-
Notifications
You must be signed in to change notification settings - Fork 217
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
Comments
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:
Closing as not planned. |
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 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): 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): if attrs then
child.expect_screenshot({ ignore_lines = ignore_lines })
else
child.expect_screen_lines({ ignore_lines = ignore_lines })
end
While I don’t mind the “hacks” I think it would be nice to have a In the hopes you’d reconsider this and ty for your amazing work! |
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 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. |
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).
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.
Ty @echasnovski! Absolutely no rush on this as my workaround code works great (ty @MagicDuck). |
A few other things I forgot to mention:
While reading TESTING.md I came across:
Perhaps it’s query mentioning that this can be also overcome by using the new
|
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.
The text was updated successfully, but these errors were encountered: