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.

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

APIとテストに変更がなければ、常に同じドキュメントを生成できるようにしたい(訂正版 #21

Merged

Conversation

meru-akimbo
Copy link
Contributor

変更に誤りがあったので、いったん閉じさせていただいた以下のpull requestに書いていた通りです。
#20

Test::JsonAPI::Autodoc::Responseのjsonの生成部分にcanonicalを指定しました。以下の箇所です。
https://github.com/moznion/Test-JsonAPI-Autodoc/blob/master/lib/Test/JsonAPI/Autodoc/Response.pm#L20

50回同じドキュメントを生成してみて、すべて中身が同じであったか確認するテストも作成しました。

仮に今回の修正部分が壊れても、稀にテストが通ってしまうこともある(たまたま50回連続で同じ要素順でjsonが生成された場合)のですが、性質上仕方がないかなと思っています。

何もテストないよりはましかなと.....

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.76% when pulling 806f898 on meru-akimbo:fix/enable_canonical_for_response into d2ad84d on moznion:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.76% when pulling 806f898 on meru-akimbo:fix/enable_canonical_for_response into d2ad84d on moznion:master.

@moznion
Copy link
Owner

moznion commented Feb 20, 2015

テスト50回もやる必要無くて,response parameter の個数を増やしてテストすれば良いと思うんですけどどうですか (確率的にバラけるはずなので)

@meru-akimbo
Copy link
Contributor Author

何回やっても同じドキュメントを吐くか、という意味のテストのつもりで書いていたので(最終的にやりたいのはそれだったので)そういう風にしていました。
50回分のドキュメント吐かせて比較する形でもテスト別に遅くなさそうだったので、まあ良いかなと思っていました。

入れてる変更自体はresponse parmeterの並び順が同じなるようにしただけなので、response parmeter増やしてそこを確認するだけでもことは足りそうですね。
そちらの方がよければテスト修正します。

@moznion
Copy link
Owner

moznion commented Feb 20, 2015

50回ドキュメント吐かせて,それを concat したものを比較するというのがあまり直感的では無い印象を受けました.

いずれにせよ意図を明確にするため,コメントなどが要るのではないかと思います.

@moznion
Copy link
Owner

moznion commented Feb 20, 2015

あとテストケースが50 + 1個になってしまうのであんまり良くない気がします.
本来不要なものがテストケースという形で,それも大量に存在しているとそちらにどうしても目を奪われて,本質を見失う可能性がありますので.

moznion added a commit that referenced this pull request Feb 27, 2015
…onse

Generate the identical documentations if APIs and tests are not modified
@moznion moznion merged commit 259f238 into moznion:master Feb 27, 2015
@moznion
Copy link
Owner

moznion commented Feb 27, 2015

とりあえずマージしました

@meru-akimbo
Copy link
Contributor Author

すみません、あまり時間とれず放置してしまっていました。
また時間あるときにテストの修正送らせていただくかもしれません。お手数おかけしました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants