Sarcasm / irony-mode

A C/C++ minor mode for Emacs powered by libclang
GNU General Public License v3.0
906 stars 99 forks source link

does-not-exists should error? (ongoing adventures with Debian's continuous integration) #460

Closed sten0 closed 6 years ago

sten0 commented 6 years ago

Hi Guillaume,

Thank you for your continued help debugging these self-test errors! Irony-mode is now passing for all primary architectures on the buildd network, and I'm now trying to get it to work on the LXC-powered continuous integration framework. Here's the latest log from https://ci.debian.net/data/autopkgtest/unstable/amd64/i/irony-mode/20171229_104813/log.gz 'hope it's useful in its truncated state!

autopkgtest [10:49:06]: test command1: [-----------------------
    emacs -batch -Q -l package --eval "(add-to-list 'package-directory-list \"/usr/share/emacs/site-lisp/elpa\")" --eval "(add-to-list 'package-directory-list \"/usr/share/emacs/site-lisp/elpa-src\")" -f package-initialize -L . -l server/test/elisp/irony.el -l server/test/elisp/irony-cdb-json.el -l server/test/elisp/irony-iotask.el -l debian/ert-helper.el

Everything looks good up until

Test irony/find-server-executable/does-not-exists backtrace:
  (if errorp21 nil (ert-fail (append (funcall form-description-fn-22) 
  (let ((errorp21 nil) (form-description-fn-22 (function (lambda nil f
  (let (form-description-20) (let ((errorp21 nil) (form-description-fn
  (let ((value-18 (quote ert-form-evaluation-aborted-19))) (let (form-
  (let ((fn-16 (function irony--find-server-executable)) (args-17 (lis
  (let ((irony-server-install-prefix "/does/not/exists")) (let ((fn-16
  (lambda nil (let ((irony-server-install-prefix "/does/not/exists")) 
  ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc
  ert-run-test([cl-struct-ert-test irony/find-server-executable/does-n
  ert-run-or-rerun-test([cl-struct-ert--stats t [[cl-struct-ert-test c
  ert-run-tests(t #[385 "\306\307\"\203G�\211\211G\310U\203�\211@\20
  ert-run-tests-batch(nil)
  ert-run-tests-batch-and-exit()
  eval-buffer(#<buffer  *load*> nil "/tmp/autopkgtest-lxc.8w79ard2/dow
  load-with-code-conversion("/tmp/autopkgtest-lxc.8w79ard2/downtmp/bui
  load("/tmp/autopkgtest-lxc.8w79ard2/downtmp/build.oC1/src/debian/ert
  command-line-1(("-l" "package" "--eval" "(add-to-list 'package-direc
  command-line()
  normal-top-level()
Test irony/find-server-executable/does-not-exists condition:
    (ert-test-failed
     ((should-error
       (irony--find-server-executable)
       :type 'irony-server-error)
      :form
      (irony--find-server-executable)
      :value "/usr/bin/irony-server" :fail-reason "did not signal an error"))
   FAILED  31/41  irony/find-server-executable/does-not-exists

Sadly I'm still not proficient enough in elisp to be able to tell why irony-server-error@irony.el:L582 isn't functioning as it ought to (probably with respect to the autotest infrastructure); although, I suspect the error signal is not the type that dh-elpa expects, and consequently isn't detected, and then isn't passed through to the bits of autopkgtest running outside of the container (see /tmp/autopkgtest... in the backtrace for evidence of this). From what I can tell, these are the layers: ERT tests run by emacs --batch, with output and exit codes managed by dh-elpa-test, run by autopkgtest run in a container by debci with various bits passed back to autopkgtest outside the container...presumably for logging.

I've also reproduced the issue on my own development machine with autopkgtest using the lxc backend, without the debci layer. I also removed the probably unnecessary ert-helper.el customisation for this run:

autopkgtest [19:08:15]: test command1: dh_elpa_test --autopkgtest
autopkgtest [19:08:15]: test command1: [-----------------------
        emacs -batch -Q -l package --eval "(add-to-list 'package-directory-list \"/usr/share/emacs/site-lisp/elpa\")" --eval "(add-to-list 'package-directory-list \"/usr/share/emacs/site-lisp/elpa-src\")" -f package-initialize -L . -l server/test/elisp/irony-iotask.el -l server/test/elisp/irony.el -l server/test/elisp/irony-cdb-json.el --eval \(ert-run-tests-batch-and-exit\)
Loading /tmp/autopkgtest-lxc.kade38ks/downtmp/build.gJW/real-tree/server/test/elisp/test-config.el (source)...
Loading /tmp/autopkgtest-lxc.kade38ks/downtmp/build.gJW/real-tree/server/test/elisp/test-config.el (source)...
Loading /tmp/autopkgtest-lxc.kade38ks/downtmp/build.gJW/real-tree/server/test/elisp/test-config.el (source)...
Running 41 tests (2018-01-04 00:08:15+0000)
   passed   1/41  cdb/choose-closest-path/chooses-closest

... everything looks good, but then:

   passed  30/41  irony/extract-working-directory-option/specified-2
Test irony/find-server-executable/does-not-exists backtrace:
  (if errorp133 nil (ert-fail (append (funcall form-description-fn-134
  (let ((errorp133 nil) (form-description-fn-134 (function (lambda nil
  (let (form-description-132) (let ((errorp133 nil) (form-description-
  (let ((value-130 (quote ert-form-evaluation-aborted-131))) (let (for
  (let ((fn-128 (function irony--find-server-executable)) (args-129 (l
  (let ((irony-server-install-prefix "/does/not/exists")) (let ((fn-12
  (lambda nil (let ((irony-server-install-prefix "/does/not/exists"))
  ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc
  ert-run-test([cl-struct-ert-test irony/find-server-executable/does-n
  ert-run-or-rerun-test([cl-struct-ert--stats t [[cl-struct-ert-test c
  ert-run-tests(t #[385 "\306\307\"\203G\211\211G\310U\203\211@\20
  ert-run-tests-batch(nil)
  ert-run-tests-batch-and-exit()
  eval((ert-run-tests-batch-and-exit))
  command-line-1(("-l" "package" "--eval" "(add-to-list 'package-direc
  command-line()
  normal-top-level()
Test irony/find-server-executable/does-not-exists condition:
    (ert-test-failed
     ((should-error
       (irony--find-server-executable)
       :type 'irony-server-error)
      :form
      (irony--find-server-executable)
      :value "/usr/bin/irony-server" :fail-reason "did not signal an error"))
   FAILED  31/41  irony/find-server-executable/does-not-exists
   passed  32/41  irony/split-command-line/backslash-1
   passed  33/41  irony/split-command-line/backslash-2
   passed  34/41  irony/split-command-line/end-with-space
   passed  35/41  irony/split-command-line/ill-end-quote
   passed  36/41  irony/split-command-line/just-spaces
   passed  37/41  irony/split-command-line/quotes-in-word
   passed  38/41  irony/split-command-line/space-everywhere
   passed  39/41  irony/split-command-line/start-with-quotes
   passed  40/41  irony/split-command-line/start-with-space
   passed  41/41  irony/split-command-line/with-quotes

Ran 41 tests, 40 results as expected, 1 unexpected (2018-01-04 00:08:16+0000)

1 unexpected results:
   FAILED  irony/find-server-executable/does-not-exists

dh_elpa_test: emacs -batch -Q -l package --eval "(add-to-list 'package-directory-list \"/usr/share/emacs/site-lisp/elpa\")" --eval "(add-to-list 'package-directory-list \"/usr/share/emacs/site-lisp/elpa-src\")" -f package-initialize -L . -l server/test/elisp/irony-iotask.el -l server/test/elisp/irony.el -l server/test/elisp/irony-cdb-json.el --eval \(ert-run-tests-batch-and-exit\) returned exit code 1
autopkgtest [19:08:16]: test command1: -----------------------]
autopkgtest [19:08:16]: test command1:  - - - - - - - - - - results - - - - - - - - - -
command1             FAIL non-zero exit status 2
autopkgtest [19:08:17]: @@@@@@@@@@@@@@@@@@@@ summary
command1             FAIL non-zero exit status 2

Other notes: I had to customise dh-elpa to copy the tests directory into the lxc container and the autotest protocol necessitates that the test are run against the installed package with byte-compiled libraries to more closely approximate the end-user test-case. I apologise in advance if I missed anything!

And here is the stable link to irony-mode's continuous integration status (should validate irony-mode functions correctly for Debian unstable/sid, testing, and all derivatives such as Ubuntu so long as they don't make major changes to Emacs or dh-elpa): https://ci.debian.net/packages/i/irony-mode/

sten0 commented 6 years ago

P.S. Happy new year!

Sarcasm commented 6 years ago

Hello,

Happy new year to you too.

Do you have a irony-server available in PATH by any chance, or at least in Emacs exec-path? Unfortunately, the test in its current form does not support this (will fix).

Sarcasm commented 6 years ago

I have submitted a fix, can you try it or do you need a new tag?

sten0 commented 6 years ago

Hi Guillaume,

Aha! Yes, that's exactly what it was, because the autopkgtests are structured to test the installed package rather than copies generated in the src tree. Thank you, 82ba45ec15c9011bbdf1d69cf25c8193d33c0028 fixes this.

No need for a new tagged version this time, because this fix doesn't affect licensing and is trivially easy to cherry pick or carry as a patch. Thank you for offering, I appreciate it :-)

Best Wishes, Nicholas

On 4 January 2018 at 14:11, Guillaume Papin notifications@github.com wrote:

I have submitted a fix, can you try it or do you need a new tag?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Sarcasm/irony-mode/issues/460#issuecomment-355371309, or mute the thread https://github.com/notifications/unsubscribe-auth/AK2HAZE-VA0-B6DB1ZFeXdtDCkIZvBT2ks5tHSJtgaJpZM4RScMS .

Sarcasm commented 6 years ago

Great!