Closed DamienCassou closed 1 year ago
This looks pretty good.
I want to work on a test suite. The first obvious issue is that the current functions are not test-suite-friendly: it's hard to replace the result of the command call by the output recordings.
My suggestion is to extract all the process-file
calls to separate functions so that the test suite can easily override them. Do you have a better suggestion?
I would replace bash_history
with a generate.sh
executable. What do you think?
What about naming the files like their command? For instance
apt-mark showmanual > 'apt-mark showmanual'
This could allow for a more generic test function (i.e. no need to maintain the list/map of commands/outputs, it can be deducted from the files in the /test-data/$manager
folder).
That would also decrease the cognitive burden (associating file names and commands). Insights?
I'll work on a test suite for Guix and push that as soon as possible. I'll keep it on a separate branch so that you can test with dnf (and dpkg?) and let me know if it's generic enough.
Sounds good?
The first obvious issue is that the current functions are not test-suite-friendly
more abstractions will make the code easier to test as well :-).
it's hard to replace the result of the command call by the output recordings. […] Do you have a better suggestion?
What about something like that? This temporarily replaces the
definition of process-file
by something else:
(ert-deftest helm-system-packages-dpkg-cache-names ()
(cl-letf (((symbol-function 'process-file)
(lambda (&rest rest) (insert (insert-file-contents "list-names.dpkg")))))
...)
The buttercup framework makes that kind of code more readable.
I would replace
bash_history
with agenerate.sh
executable. What do you think?
the bash_history file is only meant for you to know which file corresponds to which command. You might want to keep it for your tests, but I hadn't thought about that when I sent it. Do whatever you want with it.
What about name the files like there command?
In my opinion, files should be named the same across package
manager. E.g., emacs25-files.dpkg
, emacs25-files.guix
or
dpkg/emacs25-files
… The benefit would be that, with sufficient
abstraction, you don't have to write package manager-specific
tests. One test function will test all package managers by iterating
over files (e.g., emacs25-files.*
).
Sounds good?
yes!
I think we should not use emacs25
as reference package because it is not present on many distros (e.g. Arch Linux) and it will disappear sooner or later from all distros. What about using emacs
instead?
cl-letf
ing process-file
sounds like a good solution. That might not cut it for everything, but let's see once I'm done with the Guix test suite.
I have no experience with Buttercup. Can you weight the pros and cons?
bash_history
: well, test data must be reproducible, or we lose precious information here.
I'll rename it to generate.sh
then.
In my opinion, files should be named the same across package manager. E.g., emacs25-files.dpkg, emacs25-files.guix or dpkg/emacs25-files… The benefit would be that, with sufficient abstraction, you don't have to write package manager-specific tests. One test function will test all package managers by iterating over files (e.g., emacs25-files.*).
I also want to go down the way of abstraction, maybe you misunderstood what I meant previously: if we name the files likes the process-file
string, then there is no need to maintain a map "command string" -> "test output" because the test suite can infer it directly. For instance
;; Define the `manager' somewhere...
(ert-deftest helm-system-packages-dpkg-cache-names ()
(cl-letf (((symbol-function 'process-file)
(lambda (&rest rest) (insert
(insert-file-contents
(expand-file-name
(mapconcat 'identity rest " ")
(helm-system-packages-manager-name manager)))))))
...)
The only difference with your approach is that we don't have to explicitly name the test file.
What about using
emacs
instead?
sure
Can you weight the pros and cons?
pros : syntax is much cleaner when you stub functions
cons : you introduce a new dependency which brings its own learning curve and maintainance burden
I tend to stay with ert as much as I can these days for simplicify and also try to reduce my usage of stubs which reduces the need for buttercup. Nevertheless, butercup is a nice framework if you are onto these things.
if we name the files likes the
process-file
string
nice! I didn't think about that solution. This is elegant. The only
problem I see is if the command line can't easily be converted to a
file name: e.g., WinPkgManager /install Emacs
. Windows command-line
tools sometimes use '/' for options. You might also have problems if
the module uses unix-tools to quickly adapt the output of their
package manager: e.g., pkg list emacs | sed s/…/…/
.
Can you update with emacs
instead of emacs25
? Then I'll start working on a test suite.
buttercup: OK, I'll keep that in mind and for now let's go with ERT.
The only problem I see is if the command line can't easily be converted to a file name: e.g., WinPkgManager /install Emacs.
Well, Windows is not really a priority :p But even then, we make sure the way the test suite is designed allows for specialization.
Is WinPkgManager /intall emacs
a real thing? O.o
Can you update with emacs instead of emacs25?
done. I also renamed the generation script.
Windows is not really a priority. Is WinPkgManager /intall emacs a real thing?
I made that up. The purpose of this example was to tell you that not every command can be translated to a filename.
The purpose of this example was to tell you that not every command can be translated to a filename.
Absolutely, but all of them can so far so I guess it's good enough. The few that cannot will have to use specialized functions as you initially suggested, which is fine too.
I'm not sure I understand what you mean. Here is the output of the command for both emacs and emacs25.
$ apt-cache rdepends emacs
emacs
Reverse Depends:
elpa-ace-link
elpa-zzz-to-char
elpa-zzz-to-char
elpa-zzz-to-char
elpa-ztree
elpa-ztree
elpa-ztree
elpa-zenburn-theme
elpa-zenburn-theme
elpa-zenburn-theme
elpa-yasnippet
|yaml-mode
|x-face-el
elpa-ws-butler
elpa-ws-butler
elpa-ws-butler
|wl-beta
|wl
|whizzytex
|w3m-el-snapshot
|w3m-el
elpa-vimish-fold
elpa-vimish-fold
elpa-vimish-fold
|verilog-mode
elpa-vala-mode
elpa-use-package
elpa-use-package
elpa-use-package
|twittering-mode
|tuareg-mode
|tiarra-conf-el
|tdiary-mode
|supercollider-emacs
elpa-solarized-theme
elpa-solarized-theme
elpa-solarized-theme
elpa-sml-mode
elpa-smex
elpa-smex
elpa-smex
|skktools
singular-ui-emacs
silversearcher-ag-el
elpa-shut-up
elpa-shut-up
elpa-shut-up
elpa-seq
elpa-seq
|semi
|select-xface
|cmuscheme48-el
s-el
|riece
elpa-restart-emacs
elpa-restart-emacs
elpa-restart-emacs
elpa-recursive-narrow
elpa-recursive-narrow
elpa-recursive-narrow
|rdtool-elisp
elpa-rainbow-mode
elpa-rainbow-mode
elpa-rainbow-mode
elpa-rainbow-delimiters
elpa-rainbow-delimiters
elpa-rainbow-delimiters
|rail
|rabbit-mode
rabbit
|quilt-el
elpa-projectile
elpa-projectile
elpa-projectile
|post-el
elpa-popup
elpa-popup
elpa-popup
elpa-pkg-info
elpa-pkg-info
elpa-pkg-info
elpa-perspective
elpa-perspective
elpa-perspective
elpa-persp-projectile
elpa-persp-projectile
elpa-persp-projectile
elpa-parsebib
elpa-parsebib
elpa-parsebib
elpa-paredit
elpa-paredit
elpa-paredit
elpa-org-bullets
elpa-org-bullets
elpa-org-bullets
|ocaml-mode
|navi2ch
elpa-muse
elpa-muse
|mu-cite
|mpg123-el
|emacs-mozc
elpa-monokai-theme
elpa-monokai-theme
elpa-monokai-theme
|mhc
|mh-e
|mew-beta
|mew
|elpa-markdown-mode
|malaga-mode
elpa-makey
elpa-makey
elpa-makey
|lyskom-elisp-client
|lsdb
|lookup-el
|lisaac-mode
elpa-linum-relative
elpa-linum-relative
elpa-linum-relative
|liece
elpa-let-alist
elpa-let-alist
elpa-js2-mode
|initz
|ilisp
elpa-iedit
elpa-iedit
elpa-iedit
elpa-ido-vertical-mode
elpa-ido-vertical-mode
elpa-ido-vertical-mode
elpa-ido-ubiquitous
elpa-ido-ubiquitous
elpa-ido-ubiquitous
elpa-ido-completing-read+
elpa-ido-completing-read+
elpa-ido-completing-read+
elpa-ibuffer-vc
elpa-ibuffer-vc
elpa-ibuffer-vc
elpa-ibuffer-projectile
elpa-ibuffer-projectile
elpa-ibuffer-projectile
elpa-hydra
elpa-hydra
elpa-hydra
|howm
elpa-helm-projectile
elpa-helm-projectile
elpa-helm-projectile
elpa-helm-core
elpa-helm-core
elpa-helm-core
elpa-helm
elpa-helm
elpa-helm
haskell-mode
haml-elisp
gworkspace-apps-wrappers
|gri-el
elpa-goto-chg
elpa-goto-chg
elpa-goto-chg
|goby
|golang-mode
|gnuserv
|gnu-smalltalk-el
elpa-git-timemachine
elpa-git-timemachine
elpa-git-timemachine
|git-el
|gettext-el
|elpa-geiser
elpa-fsm
elpa-fsm
elpa-fsm
|frama-c
elpa-flycheck
elpa-flycheck
elpa-flycheck
elpa-flx-ido
elpa-flx-ido
elpa-flx-ido
elpa-flx
elpa-flx
elpa-flx
|flim
elpa-fill-column-indicator
elpa-fill-column-indicator
elpa-fill-column-indicator
elpa-f
elpa-f
elpa-f
elpa-expand-region
|eweouz
elpa-evil-paredit
elpa-evil-paredit
elpa-evil-paredit
elpa-evil
elpa-evil
elpa-evil
elpa-eshell-up
elpa-eshell-up
elpa-eshell-up
elpa-ert-async
elpa-ert-async
elpa-ert-async
elpa-epl
elpa-epl
elpa-epl
|emms
emacspeak
elpa-simple-httpd
elpa-simple-httpd
elpa-simple-httpd
elpa-tablist
elpa-python-environment
elpa-python-environment
elpa-python-environment
elpa-powerline
elpa-powerline
elpa-powerline
elpa-pdf-tools
elpa-noflet
elpa-noflet
elpa-noflet
elpa-jabber
elpa-jabber
elpa-jabber
elpa-highlight-indentation
elpa-highlight-indentation
|emacs-goodies-el
|dpkg-dev-el
|devscripts-el
|debian-el
elpa-epc
elpa-epc
elpa-epc
elpa-deferred
elpa-deferred
elpa-deferred
elpa-concurrent
elpa-concurrent
elpa-concurrent
elpa-ctable
elpa-ctable
elpa-ctable
elpa-buttercup
elpa-buttercup
elpa-async
elpa-async
elpa-async
elpa-anzu
elpa-anzu
elpa-anzu
elpa-undo-tree
elpa-undo-tree
elpa-undo-tree
elpa-rust-mode
elpa-rust-mode
elpa-elisp-slime-nav
elpa-elisp-slime-nav
elpa-elisp-slime-nav
elpa-elfeed-web
elpa-elfeed-web
elpa-elfeed-web
elpa-elfeed
elpa-elfeed
elpa-elfeed
|eldav
|el-get
elpa-editorconfig
elpa-editorconfig
|edict-el
|edb
|ecb
elpa-ebib
elpa-ebib
elpa-ebib
elpa-discover-my-major
elpa-discover-my-major
elpa-discover-my-major
elpa-diminish
elpa-diminish
elpa-diminish
|dictionary-el
|develock-el
education-common
|ddskk
|ddskk
|cxref-emacs
|cvc3-el
|crypt++el
|conkeror
|elpa-company
|migemo-el
elpa-clues-theme
elpa-clues-theme
elpa-clues-theme
|cdargs
|c-sig
elpa-bind-key
elpa-bind-key
elpa-bind-key
|bhl
elpa-beacon
elpa-beacon
elpa-beacon
|bbdb3
elpa-avy
elpa-avy
elpa-avy
|auto-install-el
|auto-complete-el
code-aster-run
|apel
|anything-el
|anthy-el
elpa-aggressive-indent
elpa-aggressive-indent
elpa-aggressive-indent
elpa-agda2-mode
elpa-agda2-mode
elpa-ace-window
elpa-ace-window
elpa-ace-window
elpa-ace-link
elpa-ace-link
$ apt-cache rdepends emacs25
emacs25
Reverse Depends:
|auctex
emacs25-lucid
emacs25-nox
elpa-ztree
emacs25-lucid
emacs25-nox
|yatex
emacs25-lucid
emacs25-nox
elpa-yasnippet
emacs25-lucid
emacs25-nox
|w3m-el-snapshot
emacs25-lucid
emacs25-nox
|w3m-el
emacs25-lucid
emacs25-nox
twittering-mode
emacs25-lucid
emacs25-nox
|speechd-el
emacs25-lucid
emacs25-nox
|slime
emacs25-lucid
emacs25-nox
|sepia
emacs25-lucid
emacs25-nox
|search-citeseer
emacs25-lucid
emacs25-nox
elpa-recursive-narrow
emacs25-lucid
emacs25-nox
|psgml
emacs25-lucid
emacs25-nox
proofgeneral
emacs25-lucid
emacs25-nox
|org-mode
emacs25-lucid
emacs25-nox
elpa-muse
emacs25-lucid
emacs25-nox
elpa-monokai-theme
emacs25-lucid
emacs25-nox
|minlog
emacs25-lucid
emacs25-nox
|mhc
emacs25-lucid
emacs25-nox
|mh-e
emacs25-lucid
emacs25-nox
|mew-beta
emacs25-lucid
emacs25-nox
|mew
emacs25-lucid
emacs25-nox
elpa-makey
emacs25-lucid
emacs25-nox
|lookup-el
emacs25-lucid
emacs25-nox
|ilisp
emacs25-lucid
emacs25-nox
elpa-iedit
emacs25-lucid
emacs25-nox
elpa-ido-vertical-mode
emacs25-lucid
emacs25-nox
elpa-ido-ubiquitous
emacs25-lucid
emacs25-nox
elpa-ido-completing-read+
emacs25-lucid
emacs25-nox
elpa-hydra
emacs25-lucid
emacs25-nox
elpa-helm-core
emacs25-lucid
emacs25-nox
elpa-helm
emacs25-lucid
emacs25-nox
|haskell-mode
emacs25-lucid
emacs25-nox
|goby
emacs25-lucid
emacs25-nox
elpa-git-timemachine
emacs25-lucid
emacs25-nox
elpa-fsm
emacs25-lucid
emacs25-nox
elpa-expand-region
emacs25-lucid
emacs25-nox
emacs25-nox
emacs25-lucid
emacs25-nox
emacs25-nox
emacs25-lucid
emacs25-lucid
emacs25-lucid
emacs25-nox
emacs25-lucid
emacs25-nox
|emacs-window-layout
emacs25-lucid
emacs25-nox
elpa-jabber
emacs25-lucid
emacs25-nox
elpa-deferred
emacs25-lucid
emacs25-nox
elpa-concurrent
emacs25-lucid
emacs25-nox
|emacs-calfw
emacs25-lucid
emacs25-nox
elpa-anzu
emacs25-lucid
emacs25-nox
|egg
emacs25-lucid
emacs25-nox
|e2wm
emacs25-lucid
emacs25-nox
elpa-discover-my-major
emacs25-lucid
emacs25-nox
|dh-elpa
emacs25-lucid
emacs25-nox
|ddskk
emacs25-lucid
emacs25-nox
|ddskk
emacs25-lucid
emacs25-nox
elpa-clues-theme
emacs25-lucid
emacs25-nox
elpa-avy
emacs25-lucid
emacs25-nox
auto-install-el
emacs25-lucid
emacs25-nox
|auto-complete-el
emacs25-lucid
emacs25-nox
I meant: can you check if the Helm action for displaying reverse dependencies works with dpkg?
There are indeed problems:
|
That's what I thought. When I revamped dpkg.el, I only had access to a dpkg machine for a short while, so it was only briefly tested, and I assume that I tested it on a less complicated package.
Which brings up another issue for testing: we might need several samples of the same command. For instance
apt-cache rdepends tzdata
, apt-cache rdepends emacs
and apt-cache rdepends libpng
.
I'm closing my old PRs :-). Feel free to reopen if you plan to do anything about it.
Damien Cassou @.***> writes:
- ( ) text/plain (*) text/html
I'm closing my old PRs :-). Feel free to reopen if you plan to do anything about it.
Thanks, but I am afraid this package is no more maintained.
— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.*Message ID: @.***>
-- Thierry
This adds some test data for dpkg. I can produce data for other system packages, but I'm waiting for you to make use of that first.