Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace vital spec with themis #207

Closed
29 tasks done
ujihisa opened this issue Sep 8, 2014 · 49 comments
Closed
29 tasks done

Replace vital spec with themis #207

ujihisa opened this issue Sep 8, 2014 · 49 comments
Assignees
@thinca

Comments

@ujihisa
Copy link
Member

ujihisa commented Sep 8, 2014

I discovered that a Vimmer made a good Vim script testing framework which is called themis https://github.com/thinca/vim-themis

I suggest that we abandon vital spec and use themis in vital as the standard testing framework.

TODOs if we decide to use themis

  • Introduce themis
  • Fix ci to use both vital spec and themis for graceful migration
  • Replace all existing specs with themis
  • Remove vital spec, and fix ci only to use themis

Migration List

  • bitwise.vim → thinca
  • data/string.vim → ujihisa
  • data/list.vim → ujihisa
  • data/base64.vim → mattn
  • data/lazylist.vim → ujihisa
  • data/dict.vim → thinca
  • data/ordered_set.vim → tyru
  • database/sqlite.vim → ujihisa
  • date_time.vim → thinca
  • interpreter/brainf__k.vim → ujihisa
  • locale/message.vim → thinca
  • math.vim → ujihisa
  • option_parser.vim → rhysd
  • prelude.vim → ujihisa
  • process.vim → tyru
  • process_manager.vim → ujihisa
  • random.vim → thinca
  • random/xor128.vim → Linda_pp
  • text/lexer.vim → rbtnn
  • text/table.vim → kamichidu
  • text/ltsv.vim → thinca
  • text/csv.vim → tyru
  • vim/buffer.vim → thinca
  • vim/message.vim → mattn
  • vim/search.vim → ujihisa
  • vim/compat.vim → mattn
  • web/http.vim → mattn
  • web/json.vim → mattn
  • web/uri.vim → mattn

No test

  • assertion.vim rbtnn
  • data/collection.vim ujihisa
  • data/tree.vim rbtnn
  • experimental/functor.vim tyru
  • lua/prelude.vim ujihisa
  • mapping.vim tyru
  • system/cache.vim ujihisa
  • system/file.vim Shougo
  • system/filepath.vim thinca
  • text/parser.vim rbtnn
  • vim/buffer_manager.vim tyru
  • web/html.vim mattn
  • web/xml mattn
This was referenced Sep 8, 2014
Closed
Closed
Closed
Merged
Closed
@Shougo
Copy link
Member

Shougo commented Sep 8, 2014

👍

3 similar comments
@deris
Copy link
Member

deris commented Sep 8, 2014

👍

@rhysd
Copy link
Contributor

rhysd commented Sep 8, 2014

👍

@rbtnn
Copy link
Contributor

rbtnn commented Sep 8, 2014

👍

@ujihisa
Copy link
Member Author

ujihisa commented Sep 8, 2014

Let's deal with it. I've assigned to the person who must know the best.

@kamichidu
Copy link
Contributor

👍

1 similar comment
@mattn
Copy link
Member

mattn commented Sep 9, 2014

👍

@tyru
Copy link
Member

tyru commented Sep 9, 2014

the person who must know the best.

@thinca san is the best?
Is themis now already mergeable into vital ?

(Somehow @thinca san's work might be too heavy now.
I think that the number of reviewer of one pull req is one person enough to
merge it.
And we should automate a code review more by static analysis)

@ujihisa
Copy link
Member Author

ujihisa commented Sep 9, 2014

@tyru I agree. We can be the 2nd thincas.

@mattn
Copy link
Member

mattn commented Sep 9, 2014

See 究極のthinca

@rhysd
Copy link
Contributor

rhysd commented Sep 9, 2014

themis って vital.vim の中に submodule とかで組み込んでしまうのが良いんでしょうか．

@kamichidu
Copy link
Contributor

See also thincaできるようになったぞ！

@kamichidu
Copy link
Contributor

@tyru @rhysd
単にテスト時にcloneするだけじゃダメですか？
CI用のsh (batもあるとベター) にcloneするスクリプト書いておいて、テスト時はそれを叩けばいいだけ、みたいな。

@rhysd
Copy link
Contributor

rhysd commented Sep 9, 2014

単にテスト時にcloneするだけじゃダメですか？

themis.vim の後方互換性が完全に保たれるならそれで問題ないです

@kamichidu
Copy link
Contributor

themis.vim の後方互換性が完全に保たれるならそれで問題ないです

後方互換性がなくなる段階で、vitalに組み込むか、バージョンをロックするか、最新に追従するかを決めれば良いと思います。
なんでかというと、テストはあくまでテストなので、通常の利用者がcloneした段階で一緒にcloneされるのはアレ感があります。
バグ等がある場合も、最新版で試すのが基本だと思うので、過去時点のバージョンについて互換性を気にするのもコストの割に、な気がします。
(あくまで私の一意見。議論はあって然るべきかとも思います。)

@thinca
Copy link
Member

thinca commented Sep 9, 2014

今後完全に互換を保つ保証はないですが(エンバグするかもしれないし)、clone する時に --branch v1.2 みたいにタグを指定してもらえればそこは大丈夫かと。

@tyru
Copy link
Member

tyru commented Sep 9, 2014

タグ指定すれば双方にとって安心ですね。
@thinca さんは気兼ねなく改良するコミットができるし
vitalのCIにも影響ないし。

個人的にはthemisには互換性考えずに便利だと思った機能をガンガン取り入れてほしいです！
2014/09/09 11:16 "thinca" notifications@github.com:

今後完全に互換を保つ保証はないですが(エンバグするかもしれないし)、clone する時に --branch v1.2
みたいにタグを指定してもらえればそこは大丈夫かと。

—
Reply to this email directly or view it on GitHub
#207 (comment).

@deris
Copy link
Member

deris commented Sep 9, 2014

まさにVim-Vimの関係

@rhysd
Copy link
Contributor

rhysd commented Sep 9, 2014

今後完全に互換を保つ保証はないですが(エンバグするかもしれないし)、clone する時に --branch v1.2 みたいにタグを指定してもらえればそこは大丈夫かと。

了解です．
では，vital.vim 全体で themis.vim のバージョンをブランチ指定（現行は1.2？）で固定するという運用でいくわけですね．

@rbtnn
Copy link
Contributor

rbtnn commented Sep 9, 2014

このthemis計画はいつから実施するんでしょう？
specはいつまで残しておく？
themis関連のテストファイルはどういうファイル名で階層はどうするのか？

とりあえず、疑問に思ったことを挙げてみました。

@thinca
Copy link
Member

thinca commented Sep 9, 2014

このthemis計画はいつから実施するんでしょう？

移行タイミングですが、個人的には themis.vim 1.3 がリリースできたからがいいかな、と思っています。
ただ、なるべくリリースは急ぎますが、いつまでにできる、と言った約束はできないので、待てないのであればもう始めても良いです。
1.3 からにする根拠ですが、style と言う機能をいれて複数のスタイルでテストが書けるようになります。
1.3 の段階では、今まで普通に使っていた basic と言うスタイルの他に、vimspec と言うスタイルを追加する予定です。以下の様な感じでテストが書けるようにする予定です(まだ予定)。(一部抜粋につき未定義の関数とか使ってます)

" test/helper/assert.vimspec
Describe helper-assert

  Describe .equals()
    It checks actual equals to expect
      call s:assert.equals(5 + 5, 10)
      call s:assert.equals(2.0 + 5.0, 7)
      call s:assert.equals('hoge' . 'huga', 'hogehuga')
      call s:assert.equals('10', 10)
      call s:assert.equals(range(3), [0, 1, 2])
    End
    It throws a report when values are not equivalent
      call s:check_throw('equals', [1 + 1, 11], 'The equivalent values were expected')
      call s:check_throw('equals', ['hoge', 'HOGE'], 'The equivalent values were expected')
    End
    It returns truthy value when check was successful
      call s:assert.truthy(s:assert.equals(1, 1))
    End
    It accepts an optional message
      call s:assert.equals(5 + 5, 10, 'error message')
      call s:check_throw('equals', [1 + 1, 11, 'error message'], 'error message')
    End
  End

End

こちらの方が既存のコードから移行し易いかと思います。

specはいつまで残しておく？

@ujihisa さんも言ってますが、移行が始まったら1つずつ少しずつ移行していき、移行中は両方のテストを走らせるようにして、全て移行が完了したら古い方を消してしまう感じになると思います。

themis関連のテストファイルはどういうファイル名で階層はどうするのか？

ちゃんとは考えてないですが、モジュール側と同じ構造にして、1ファイル1モジュールのテストでいいかなぁ、と。core とか vitalizer は別途必要ですが、モジュール名と衝突はしないと思うので大丈夫かと。

@rhysd
Copy link
Contributor

rhysd commented Sep 9, 2014

vimspec++

@tyru
Copy link
Member

tyru commented Sep 9, 2014

おおお、vimspec++

@deris
Copy link
Member

deris commented Sep 9, 2014

vimspec++

@ujihisa
Copy link
Member Author

ujihisa commented Sep 11, 2014

One of the most important feature what we need is to test with vimproc, since couple of modules depend on vimproc. vital spec doesn't support vimproc, so there's no test available for Process and ProcessManager so far.

@syngan
Copy link
Contributor

syngan commented Sep 11, 2014

vital spec doesn't support vimproc,

https://github.com/vim-jp/vital.vim/blob/master/.travis.yml#L14

今も vimproc が必要となるテストも動いているのでは?

@ujihisa
Copy link
Member Author

ujihisa commented Sep 12, 2014

syngan

so handy

👍

@thinca
Copy link
Member

thinca commented Oct 5, 2014

themis 1.3 リリースしました！

@ujihisa
Copy link
Member Author

ujihisa commented Oct 5, 2014

1.3

Can't wait to see themis for vital!

@thinca
Copy link
Member

thinca commented Oct 6, 2014

You can do it!

@rhysd
Copy link
Contributor

rhysd commented Oct 6, 2014

++@thinca;

とりあえず移行すべきテストのリストをトップに書いてみました．移行したらチェック入れていく感じでどうでしょうか？
移行したテストの置き場所と travis の設定必要ですね．

@thinca
Copy link
Member

thinca commented Oct 6, 2014

themis.vim は複数のやり方でテストが書けるので、どのスタイル、ヘルパを使うのかを、全体で統一してしまうか、もしくはモジュール毎にメンテナが自由にしていいか、決めておきたいところです。

@thinca thinca mentioned this issue Oct 6, 2014
Merged
@rhysd
Copy link
Contributor

rhysd commented Oct 7, 2014

別段縛る理由があるのでなければ，各メンテナ任せで良いのではないでしょうか

@thinca
Copy link
Member

thinca commented Oct 7, 2014

各メンテナ任せと言う感じで行きましょう。皆さんどんどんやってくれてよいですよ！

@osyo-manga
Copy link

別段縛る理由があるのでなければ，各メンテナ任せで良いのではないでしょうか

本件とは全く関係ないのですが、『各モジュールに対するメンテナの一覧』みたいなページってあるのでしたっけ。
ないのであればつくっておくと便利そうな気がします！

@thinca
Copy link
Member

thinca commented Oct 7, 2014

お願いします！

@rhysd
Copy link
Contributor

rhysd commented Oct 7, 2014

https://gist.github.com/rhysd/1c32685942d4e8cfbe5d

適当にスクリプトで取りました．

vital-assertion.txt             Maintainer: rbtnn
vital-bitwise.txt               Maintainer: thinca
vital-data-base64.txt           Maintainer: mattn
vital-data-collection.txt       Maintainer: ujihisa
vital-data-dict.txt             Maintainer: thinca
vital-data-lazylist.txt         Maintainer: ujihisa
vital-data-list.txt             Maintainer: ujihisa
vital-data-ordered_set.txt      Maintainer: tyru
vital-data-string.txt           Maintainer: ujihisa
vital-data-tree.txt             Maintainer: rbtnn
vital-database-sqlite.txt       Maintainer: ujihisa
vital-date_time.txt             Maintainer: thinca
vital-experimental-functor.txt  Maintainer: tyru
vital-interpreter-brainf__k.txt Maintainer: ujihisa
vital-locale-message.txt        Maintainer: thinca
vital-lua-prelude.txt           Maintainer: ujihisa
vital-mapping.txt               Maintainer: tyru
vital-math.txt                  Maintainer: ujihisa
vital-option_parser.txt         Maintainer: rhysd
vital-prelude.txt               Maintainer: ujihisa
vital-process.txt               Maintainer: tyru
vital-process_manager.txt       Maintainer: ujihisa
vital-random-mt19937ar.txt      Maintainer: Linda_pp
vital-random-xor128.txt         Maintainer: Linda_pp
vital-random.txt                Maintainer: thinca
vital-system-cache.txt          Maintainer: ujihisa
vital-system-file.txt           Maintainer: Shougo
vital-system-filepath.txt       Maintainer: thinca
vital-text-csv.txt              Maintainer: tyru
vital-text-lexer.txt            Maintainer: rbtnn
vital-text-ltsv.txt             Maintainer: thinca
vital-text-parser.txt           Maintainer: rbtnn
vital-text-table.txt            Maintainer: kamichidu
vital-vim-buffer.txt            Maintainer: thinca
vital-vim-buffer_manager.txt    Maintainer: tyru
vital-vim-compat.txt            Maintainer: mattn
vital-vim-message.txt           Maintainer: mattn
vital-web-html.txt              Maintainer: mattn
vital-web-http.txt              Maintainer: mattn
vital-web-json.txt
vital-web-xml.txt

（ちょい整形＆メールアドレス抜きました）

@rhysd
Copy link
Contributor

rhysd commented Oct 7, 2014

トップのリスト更新しました．（さっきのリストと合わせて適当にスクリプトでつくって少し手で修正したので，間違っていたらすみません）

@ujihisa
Copy link
Member Author

ujihisa commented Oct 7, 2014

cf0376e

Did the commit message prefix have to be "Bitwise: " instead of "test: "?

@thinca
Copy link
Member

thinca commented Oct 10, 2014

そこは完全に "Bitwise:" にすべきでしたね。ミスりました。

@thinca
Copy link
Member

thinca commented Mar 22, 2015

時間あるときにちまちまと作業しています。できるだけ早くこの Issue 閉じたい…!!

@rhysd
Copy link
Contributor

rhysd commented Mar 22, 2015

Web モジュールちまちまやってます．

@rhysd
Copy link
Contributor

rhysd commented Mar 22, 2015

Web モジュールちまちまやってます．

Web と Data の残りやっつけました．

@mattn
Copy link
Member

mattn commented Mar 23, 2015

犬がとうございます。

@rhysd
Copy link
Contributor

rhysd commented Mar 23, 2015

💪🐶

@rhysd
Copy link
Contributor

rhysd commented Mar 28, 2015

本日移行終わったモジュールにチェック入れておきました．
後は ProcessManager だけですね（ﾁﾗｯﾁﾗｯ

@ujihisa
Copy link
Member Author

ujihisa commented Apr 9, 2015

後は ProcessManager だけですね（ﾁﾗｯﾁﾗｯ

ProcessManagerは #278 のあとdeprecatedになる予定なので、これでもうバッチリ説浮上かな?

@ujihisa ujihisa closed this as completed Apr 9, 2015
@ujihisa
Copy link
Member Author

ujihisa commented Apr 9, 2015

@rhysd++ 614fc50...cd2d46a

@rhysd rhysd mentioned this issue Apr 9, 2015
Merged
@rhysd
Copy link
Contributor

rhysd commented Apr 9, 2015

🎉

rhysd referenced this issue Apr 22, 2015
@ujihisa
Math: themis style conversion
2ba6456
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@thinca thinca

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests