[Notes] remove mini.test hacks

[phanen]

mini.test add a new ignore_{line,attr} feature. But ignore_{line,attr} skipped only the checking but not screenshot generating.

Actually with ea6d8ce screenshots generating is also skipped. So it need to be added back... (And this seems not necessary to have https://github.com/phanen/fzf-lua/blob/109d75dc6bf36ec2c5e3671969c46595500a5376/tests/file/ui_spec.lua#L237-L241), we can just auto generating screenshot).

Another OT problem: files number increased when new files/screenshots created. It can be annoying when there're too many test files...we have to review each one's changes when update screenshots.

Solution:

• ignore_text. But this don't avoid screenshots updating? Also this don't compare prompt/query/spinner.
• --no-info/--info=xxx we can hide/move the file number but this also affect the spinner (although it's not very useful? spinner in screenshot can be used to determine if fzf terminal buffer is freezed for some reason but neovim is not freezed... but it's not really necessary).

[ibhagwan]

Actually with ea6d8ce screenshots generating is also skipped. So it need to be added back...

What do you mean added back? The tests currently pass with the changes so maybe we just leave the hack as is or we’ll be back to empty attr screenshots?

Another OT problem: files number increased when new files/screenshots created. It can be annoying when there're too many test files...we have to review each one's changes when update screenshots.

Not a real issue IMHO, when we add a test file it’s good to remember to update the file number, also verifies everything is working as intended.

--no-info/--info=xxx

I prefer to leave the info, this way I know if there’s an issue with file count / early termination, etc.

[phanen]

What do you mean added back? The tests currently pass with the changes so maybe we just leave the hack as is or we’ll be back to empty attr screenshots?

If we remove our version screenshot_read, then it will be https://github.com/echasnovski/mini.nvim/blob/de0a88d925c8e69fdbb498141207ee43504e5cd1/lua/mini/test.lua#L2317-L2330. So we need at least pad empty attr lines and its ruler line.

Attr is always generated (just not checked). So to avoid manually delete attr lines each time generating new screenshot file it's still impossible to remove test/screenshot.lua

[ibhagwan]

What do you mean added back? The tests currently pass with the changes so maybe we just leave the hack as is or we’ll be back to empty attr screenshots?

If we remove our version screenshot_read, then it will be https://github.com/echasnovski/mini.nvim/blob/de0a88d925c8e69fdbb498141207ee43504e5cd1/lua/mini/test.lua#L2317-L2330. So we need at least pad empty attr lines and its ruler line.

Attr is always generated (just not checked). So to avoid manually delete attr lines each time generating new screenshot file it's still impossible to remove test/screenshot.lua

We can just leave it as is then, mini is very stable and carful with upstream changes I don’t suspect it will break anytime soon, 8c8845c should be enough then.

[phanen]

Anyway since all test passed now maybe left it there currently... I believed mini.test rarely have large diff changes since the author already maintain 40+ plugins...

[ibhagwan]

Anyway since all test passed now maybe left it there currently... I believed mini.test rarely have large diff changes since the author already maintain 40+ plugins...

Author is also amazing and most responsible so I don't suspect many breakages :-)

Ty @phanen for your diligence on this and ty @echasnovski for your amazing work always 🙏 ❤️

[echasnovski]

Author is also amazing and most responsible so I don't suspect many breakages :-)

I contemplated several months whether I should soft deprecate ignore_lines, so yeah I try :)

[echasnovski]

Sorry, I don't really have full context, but here is what I've gathered:

• 'fzf-lua' tests basically have own concept of "screenshot", which is basically a text part of screenshots in 'mini.test'.
• To account for that, there is also custom expect.reference_screenshot() implementation.

My first instinct here would be to suggestion to just switch to 'mini.test' screenshots and have a expect.reference_screenshot() wrapper that uses MiniTest one with ignore_attr = true. Ignoring lines is then the same as ignore_text (which is what is done now). Yes this will generate new screenshots now and yes this might result in extra diff in attr part if screenshots are regenerated in the future. But judging by the number of screenshots, this should not be much of a burden.

[ibhagwan]

Sorry, I don't really have full context, but here is what I've gathered:

• 'fzf-lua' tests basically have own concept of "screenshot", which is basically a text part of screenshots in 'mini.test'.
• To account for that, there is also custom expect.reference_screenshot() implementation.

My first instinct here would be to suggestion to just switch to 'mini.test' screenshots and have a expect.reference_screenshot() wrapper that uses MiniTest one with ignore_attr = true. Ignoring lines is then the same as ignore_text (which is what is done now). Yes this will generate new screenshots now and yes this might result in extra diff in attr part if screenshots are regenerated in the future. But judging by the number of screenshots, this should not be much of a burden.

All correct, @phanen intended to remove that exact part of the empty attr lines (when applicable) so it’s easier to see what went wrong.

[phanen]

yeah it's not a big problem...

btw I just found grug-far also remove empty lines...
MagicDuck/grug-far.nvim@e310ee5