remove handwritten bash/zsh shell completions, fixes #9178#9356
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9356 +/- ##
==========================================
+ Coverage 76.03% 76.44% +0.40%
==========================================
Files 85 85
Lines 14744 14800 +56
Branches 2194 2213 +19
==========================================
+ Hits 11211 11314 +103
+ Misses 2856 2808 -48
- Partials 677 678 +1 ☔ View full report in Codecov by Sentry. |
mr-raj12
force-pushed
the
remove-handwritten-shell-completions
branch
from
9ebf1a9 to
895a048
Compare
ThomasWaldmann
left a comment
ThomasWaldmann
left a comment
There was a problem hiding this comment.
thanks for the PR.
the problem with these tests is that many of them do not really test if the completions are working, but just check "SEARCHTERM in SCRIPT".
also, that list with all commands and options will get out of sync with the code pretty soon.
so i'ld suggest to remove all the not that meaningful and hard to maintain tests. guess the script size test and the "does the shell load it" are short and meaningful.
tests that test the borg-special features (like archive id completion) would be good.
we could also check how much tests the shtab project has, so we do not test what they already have tested.
the idea with the ticket you addressed was that users actually test and use the completions and make sure they actually work good enough, but these tests can't do that.
Remove the handwritten bash and zsh shell completion scripts now that auto-generated completions via borg completion bash/zsh (powered by shtab, borgbackup#9172) are tested and working. Fish completions are kept as shtab does not yet support fish. Replace string-matching tests with focused behavior tests: script size sanity, shell syntax validation (bash -n / zsh -n), and tests that invoke the custom preamble functions in bash (sortby key dedup, filescachemode mutual exclusivity, archive name and aid: prefix completion against a real repository).
mr-raj12
force-pushed
the
remove-handwritten-shell-completions
branch
from
4c07faa to
09d2dba
Compare
- remove redundant f.flush() calls - use pytest.mark.skipif decorators instead of inline pytest.skip() - add _zsh_available() to complement _bash_available() - extract _check_shell_syntax() helper to reduce duplication
Replace separate _bash_available() and _zsh_available() with a single lru_cache-decorated cmd_available() function, per review feedback.
The archive name/aid completion tests invoke borg via bash, which exceeds 30s on Haiku OS VMs. Increase timeout to 120s.
|
Thanks! |
2 participants
Description
Remove the handwritten bash and zsh shell completion scripts now that auto-generated completions via
borg completion bash/zsh(powered by shtab, #9172) are tested and working.Fish completions are kept - shtab does not yet support fish.
Fixes #9178
Changes
scripts/shell_completions/bash/borgborg completion bashscripts/shell_completions/zsh/_borgborg completion zshsrc/borg/testsuite/archiver/completion_cmd_test.pysrc/borg/testsuite/shell_completions_test.pydocs/changes.rstTest coverage (29 tests)
bash -n/zsh -nborg repo-listusage,compadd, registrationChecklist
docs/changes.rst)