Add basic test case. #162

Merged
merged 1 commit into from Sep 16, 2012

Projects

None yet

3 participants

@uk-ar
Member
uk-ar commented Sep 13, 2012

とりあえず、etc/test.txtを見ながらちょこっとしたテストを書いてみました。
こんな感じでいかがでしょうか?
etc/test.txtにはpopup.elのテストケースも混じっていましたが、別にした方がいいと思われます。
とは言っても、popup.elのテストは書くのが辛そうですが...

@tkf tkf commented on the diff Sep 14, 2012
etc/auto-complete-test.el
@@ -0,0 +1,52 @@
+(require 'ert)
+
+(require 'auto-complete)
+(require 'auto-complete-config)
+
+(defmacro auto-complete-test:common (&rest body)
+ (declare (indent 0) (debug t))
+ `(save-excursion
+ ;; (with-current-buffer "*scratch*"
@tkf
tkf Sep 14, 2012 Member

この行は要らないのでは?

@uk-ar
uk-ar Sep 15, 2012 Member

9行目に限れば不要です。

@tkf tkf commented on the diff Sep 14, 2012
etc/auto-complete-test.el
+ ;; (with-current-buffer "*scratch*"
+ (with-temp-buffer
+ (switch-to-buffer (current-buffer))
+ (auto-complete-mode 1)
+ (emacs-lisp-mode)
+ ,@body
+ (ac-menu-delete)
+ )))
+
+(ert-deftest auto-complete-test ()
+ (auto-complete-test:common
+ (defvar ac-source-test
+ '((candidates list "Foo" "FooBar" "Bar" "Baz" "LongLongLine")))
+ (defvar ac-source-action-test
+ '((candidates list "Action1" "Action2")
+ (action . (lambda () (message "Done!")))))
@tkf
tkf Sep 14, 2012 Member

L40 もそうですが、 message を呼ぶよりは (incf counter) などとしてあとで (should (= counter 1)) でチェックするほうが良いと思います。

また、 batch モードで起動したときに、 ERT の output に message が混ざって結果が見辛くなります。

@uk-ar
uk-ar Sep 15, 2012 Member

etc/test.txt から引っ張ったきたので、何も考えてませんでした。
確かにmessageはやめた方がいいと思います。
カウンターでも良いですが、依存関係を増やしていいならmockを使った方がいいかなと思ってます。

@tkf tkf commented on the diff Sep 14, 2012
etc/auto-complete-test.el
+ '((candidates list "Foo" "FooBar" "Bar" "Baz" "LongLongLine")))
+ (defvar ac-source-action-test
+ '((candidates list "Action1" "Action2")
+ (action . (lambda () (message "Done!")))))
+ (setq ac-sources '(ac-source-test ac-source-action-test))
+ (should-not (popup-live-p ac-menu))
+ (should (eq ac-menu nil))
+ (insert "Foo")
+ (auto-complete)
+ (execute-kbd-macro "B")
+ (ac-update-greedy)
+ (should (popup-live-p ac-menu))
+ (should (equal (popup-list ac-menu) '("FooBar")))
+ ))
+
+(ert-run-tests-interactively t)
@tkf
tkf Sep 14, 2012 Member

テストの方法は、 interactive (M-x ert) でやるのと batch でやる方法があるので、どちらでも起動出来るようにするべきだと思います。なので、この行は消すべきかと。

@uk-ar
uk-ar Sep 15, 2012 Member

おっしゃる通りだと思います。

@tkf
Member
tkf commented Sep 14, 2012

I just went through the code. (@m2ym I did the review in Japanese but if you think we should always do it in English, please tell me).

Things I am not sure:

  • I expect there will be a lot of tests. Probably we should make tests/ directory?
  • I guess we better put header comment. In that case, the copyright should be something like "Auto-complete development team"?
  • Do we want : in function/variable name? I guess using ac- or ac-testing- is better.
@tkf tkf referenced this pull request Sep 15, 2012
Open

Regression tests #150

@m2ym
Member
m2ym commented Sep 15, 2012

(@m2ym I did the review in Japanese but if you think we should always do it in English, please tell me).

No problem.

I expect there will be a lot of tests. Probably we should make tests/ directory?

Yes.

Do we want : in function/variable name? I guess using ac- or ac-testing- is better.

I prefer ac-test-.

Anyway, we can improve the tests based on this.

@m2ym
Member
m2ym commented Sep 15, 2012

@uk-ar とりあえず Collaborator に追加しておきます @tkf さんと協力してテストを充実させていただけると助かります

なお v2.0 で内部構造が劇的に変化する可能性があるので、過度に内部構造に依存しないテストが望ましいと思います。例えばユーザーのキー入力をシミュレートして、期待通りの画面表示になるか、みたいな。そういう意味では Ecukes は結構良いかもしれませんが、あれってオーバーレイを正しく認識できるのだろうか…

@tkf tkf referenced this pull request Sep 15, 2012
Merged

Travis CI #163

@tkf
Member
tkf commented Sep 15, 2012

@uk-ar 私の環境だと、以下のエラーがでてテストが通りません。

F auto-complete-test
    (ert-test-failed
     ((should
       (popup-live-p ac-menu))
      :form
      (popup-live-p nil)
      :value nil))

F auto-complete-test2
    (ert-test-failed
     ((should
       (popup-live-p ac-menu))
      :form
      (popup-live-p nil)
      :value nil))

#163 で test runner を書いたので、それを使ってクリーンな Emacs 環境下でテストが通ることを確認してもらえませんか?

修正があればこの branch にコミットするか、私の branch に対して PR 送るかして下さい。

@uk-ar uk-ar referenced this pull request in tkf/auto-complete Sep 15, 2012
Merged

Fix test initialization. #1

@uk-ar
Member
uk-ar commented Sep 15, 2012

ac-config-default が実行されていないため auto-complete-mode が活性になっていませんでした。
ひとまず ac-config-default を実行しておいて、defaultを使用しないテストを追加する段階で対応しようと思います。

@m2ym さん

なお v2.0 で内部構造が劇的に変化する可能性があるので、過度に内部構造に依存しないテストが望ましいと思います。例えばユーザーのキー入力をシミュレートして、期待通りの画面表示になるか、みたいな。

一応キー入力のシミュレートは execute-kbd-macro で実現してるので大丈夫ですが、画面表示の判定はpopup-listとかに依存してます。そこだけでもラッパーを被せておいた方がいいかもしれません...
Ecukesは仕様があまりわかってませんが、「囲う」系の処理(Step?)を記述できるのかが心配です。今は with-temp-buffer ぐらいですが、テストが増えるといろいろと使用する気がします。

@tkf
Member
tkf commented Sep 15, 2012

@uk-ar ありがとうございます。後でチェックします。

Ecukes使うときは囲う系の処理は書かないと思います。 "When I switched to the buffer" みたいな感じの step を使うはずです。全てのテストに対してテスト前後に実行するコードを指定できるので、 setup と tear down はそこでできます。

@tkf tkf commented on the diff Sep 15, 2012
etc/auto-complete-test.el
+
+(defmacro auto-complete-test:common (&rest body)
+ (declare (indent 0) (debug t))
+ `(save-excursion
+ ;; (with-current-buffer "*scratch*"
+ (with-temp-buffer
+ (switch-to-buffer (current-buffer))
+ (auto-complete-mode 1)
+ (emacs-lisp-mode)
+ ,@body
+ (ac-menu-delete)
+ )))
+
+(ert-deftest auto-complete-test ()
+ (auto-complete-test:common
+ (defvar ac-source-test
@tkf
tkf Sep 15, 2012 Member

defvar を使うと、どちらかのテストで指定した変数の値が使われなくなってしまいます。 let で指定するのが良いんじゃないでしょうか。

@tkf
tkf Sep 16, 2012 Member

#163 で修正しました

@tkf
Member
tkf commented Sep 16, 2012

@m2ym さんの言う "内部構造が劇的に変化" とは auto-complete.el についてだと思うので、 popup.el の、特にAPI関数については、ビシバシ使って良いんじゃないかと思います。

@uk-ar
Member
uk-ar commented Sep 16, 2012

popup.el の公開APIに関しては了解しました。
今後もしEcukesを使用するとしたら、let,save-excursion,with-temp-bufferあたりが問題になりそうですね。
とりあえずは気にしないでテストを書いてゆこうと思います。

@m2ym さん
優先度をつけたいので、再発しそうなバグのissue番号があるとうれしいかもです。

@m2ym
Member
m2ym commented Sep 16, 2012

@uk-ar popup.elが壊れやすいので、ひとまずpopup.elのトリビアルなケースについてテストを書いていただけると助かります。テンプレートはこんな感じになるかと思います。

(let ((popup (popup-create (point) 10 10)))
  (popup-set-list popup '("foo" "bar" "baz"))
  (popup-draw popup)
  (should (equal (buffer-contents) "foo\nbar\nbaz"))
  (popup-delete popup))
@m2ym
Member
m2ym commented Sep 16, 2012

@uk-ar popup.elのテストはpopup-elリポジトリに追加してください。 #163 を参考に同じフォーマットにしておくと Travis CI の対応が楽になると思います

@tkf tkf merged commit ec26abc into auto-complete:master Sep 16, 2012
@tkf
Member
tkf commented Sep 16, 2012

@uk-ar auto-complete/popup-el#15 で popup でも Travis CI が動くようにしました。 tests/popup-test.el 内にテストを書いていけば OK です。 クリーンな Emacs でテストするには make travis-ci (または make travis-ci EMACS=emacs-snapshot など) としてください。

もともとあった popup-test.eltests/popup-interactive-test.el に移動しました。単純に ERT を使ったテストに移植出来るかはちゃんと見てないのでわかりませんが、理想的には全部 ERT を使って書き換えたいところですね。

@uk-ar uk-ar added a commit to uk-ar/auto-complete that referenced this pull request Sep 17, 2012
@uk-ar uk-ar Add workaround for auto-complete/auto-complete#162 bug. 7c163c1
@m2ym
Member
m2ym commented Sep 17, 2012

Great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment