search

hrsh7th / nvim-cmp

A completion plugin for neovim coded in Lua.
MIT License
7.53k stars 377 forks source link

ci: fix broken tests #1729

Closed mcauley-penney closed 9 months ago

mcauley-penney commented 9 months ago

The backspace setting only accepts strings. Here, I assume that 0 was intended to set backspace to a nil value, which can be done with the empty string.

mcauley-penney commented 9 months ago

I haven't figured this out just yet but it looks like its coming from attempting to set the backspace setting in nvim-cmp/lua/cmp/utils/feedkeys_spec.lua, in the backspace test spec. I initially thought it was related to trying to set backspace to an integer instead of a string (it accepts strings) but now I'm thinking its related to the way it is being set.

Shougo commented 9 months ago

It seems still failed.

mcauley-penney commented 9 months ago

It seems still failed.

Yes, I am working on this. The test was introduced in #956. I have made changes to the test that introduced the CI error, but it seems like my fix broke another test. Either the other test is also broken (I don't think so, at this point) or modifying the backspace setting in the above mentioned file and test spec is causing problems.

To be honest, it would be nice to know what the test is trying to accomplish. I can gather from the specs what it is trying to do, but I feel like knowing what @hrsh7th wanted to do by setting the backspace opt to 0 would be useful.

Edit: Also, I probably should have worked on this locally but I wanted to get a PR going for discussion. I am currently testing it locally.

mcauley-penney commented 9 months ago

@Shougo Looking at the commit history, it appears that the repo had two broken tests. If I'm understanding correctly, one of them appears to have been introduced a long while ago. I changed both to remove errors. I believe that I fixed them in the way they were supposed to be fixed, i.e. I believe that I interpreted the intention of the tests correctly and fixed them accordingly.

Shougo commented 9 months ago

It is fixed. Thanks.

hrsh7th commented 9 months ago

Thank you!