helm-system-packages icon indicating copy to clipboard operation
helm-system-packages copied to clipboard

dpkg: add test data

Open DamienCassou opened this issue 5 years ago • 12 comments

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.

DamienCassou avatar Sep 24 '18 04:09 DamienCassou

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?

Ambrevar avatar Sep 24 '18 11:09 Ambrevar

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 a generate.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!

DamienCassou avatar Sep 24 '18 11:09 DamienCassou

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-letfing 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.

Ambrevar avatar Sep 24 '18 12:09 Ambrevar

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/…/…/.

DamienCassou avatar Sep 24 '18 13:09 DamienCassou

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.

Ambrevar avatar Sep 24 '18 14:09 Ambrevar

Is WinPkgManager /intall emacs a real thing? O.o

Ambrevar avatar Sep 24 '18 14:09 Ambrevar

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.

DamienCassou avatar Sep 25 '18 06:09 DamienCassou

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.

Ambrevar avatar Sep 25 '18 09:09 Ambrevar

I'm not sure I understand what you mean. Here is the output of the command for both emacs and emacs25.

Emacs reverse dependencies

$ 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

Emacs 25 reverse dependencies

$ 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

DamienCassou avatar Oct 05 '18 15:10 DamienCassou

I meant: can you check if the Helm action for displaying reverse dependencies works with dpkg?

Ambrevar avatar Oct 05 '18 18:10 Ambrevar

There are indeed problems:

  • some packages are shown indented
  • some packages are prefixed with |
  • there is a "Reverse Depends:` package

2018-10-10-101356_720x398_scrot 2018-10-10-101430_720x400_scrot

DamienCassou avatar Oct 10 '18 08:10 DamienCassou

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.

Ambrevar avatar Oct 10 '18 08:10 Ambrevar

I'm closing my old PRs :-). Feel free to reopen if you plan to do anything about it.

DamienCassou avatar Mar 20 '23 20:03 DamienCassou

Damien Cassou @.***> writes:

  1. ( ) 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

thierryvolpiatto avatar Mar 21 '23 05:03 thierryvolpiatto