search

scop / bash-completion

Programmable completion functions for bash
GNU General Public License v2.0
2.95k stars 380 forks source link

test(scp): Add unit tests for getting remote files #1244

Closed yedayak closed 2 months ago

yedayak commented 2 months ago

Since there doesn't seem to be any unit tests for xfuncs, I'm not sure if the naming here makes sense, or if I should make some more directories, for example a unit/xfunc or fixtures/xfunc?

I did this since there seems to be a lot of issues with this function, as seen in https://github.com/scop/bash-completion/pull/910 and https://github.com/scop/bash-completion/pull/765#issuecomment-1173097965 This should be a first good step to make fixing the issues easier IMO.

akinomyoga commented 2 months ago

I think this should be tested within test/t/test_scp.py. The test/t/unit/test_unit_*.py are the tests for the core components in bash_completion. Except for that, LGTM.

yedayak commented 2 months ago

I think this should be tested within test/t/test_scp.py. The test/t/unit/test_unit_*.py are the tests for the core components in bash_completion. Except for that, LGTM.

Moved to test/t/test_scp.py. I had to add @pytest.mark.bashcomp(ignore_env=r"^\+COMPREPLY=") to the entire class, I'm not sure if there is a better way to only apply it to the necessary functions.

akinomyoga commented 2 months ago

I'm not sure if there is a better way to only apply it to the necessary functions.

You can call bash_env.save_variable("COMPREPLY").

yedayak commented 2 months ago

I'm not sure if there is a better way to only apply it to the necessary functions.

You can call bash_env.save_variable("COMPREPLY").

This doesn't seem to work, the diff doesn't look at the saved variables, it just uses get_env() which is basically just _comp__test_get_env.

akinomyoga commented 2 months ago

I'm not sure if there is a better way to only apply it to the necessary functions.

You can call bash_env.save_variable("COMPREPLY").

This doesn't seem to work,

How have you checked that it doesn't seem to work? I now tried that, but it works.

the diff doesn't look at the saved variables, it just uses get_env() which is basically just _comp__test_get_env.

The saved variables are restored by bash_env. get_env() is called after the with statement calls bash_env_saved.__exit__() and the saved variables are restored.

yedayak commented 2 months ago

I'm not sure if there is a better way to only apply it to the necessary functions.

You can call bash_env.save_variable("COMPREPLY").

This doesn't seem to work,

How have you checked that it doesn't seem to work? I now tried that, but it works.

the diff doesn't look at the saved variables, it just uses get_env() which is basically just _comp__test_get_env.

The saved variables are restored by bash_env. get_env() is called after the with statement calls bash_env_saved.__exit__() and the saved variables are restored.

Ah I managed it, I was trying to save the variable after it was changed, which in hindsight obviously doesn't make sense :)