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

Add `be_listening.with("protocol")` matcher for Port resource. #200

Merged
merged 10 commits into from Jul 5, 2013

Conversation

Projects
None yet
3 participants
@kitak
Contributor

kitak commented Jul 4, 2013

Portリソースで使えるbe_listening.with("protocol")マッチャを追加しました。
指定されたポートが与えられたプロトコルでlistenされているかどうか確認できます。
以下が記述例になります。

describe port(80) do
  it { should be_listening.with("tcp") }
end

commandsとmatcherのテストを記述して通ることを確認しています(Solarisのcommandは実際にこれでよいかどうか確かめていないです)。

@ftnk

This comment has been minimized.

Show comment
Hide comment
@ftnk

ftnk Jul 4, 2013

Contributor

Solaris のコマンドを用意してもらえたのはうれしいのですが、
残念ながら、用意していただいたコマンドでは動作しません。

以下のようにすれば動くと思います。

    regexp = ".*\.#{port} "
    "netstat -an -P #{protocol} 2> /dev/null | egrep 'LISTEN|Idle' | grep -- #{escape(regexp)}"

Solaris 用のコマンドが必要だけどコマンドの確認ができない場合は、
raise NotImplementedError.new してもらえると、
Solaris 用のコマンドが必要だとわかるので助かります。

Contributor

ftnk commented Jul 4, 2013

Solaris のコマンドを用意してもらえたのはうれしいのですが、
残念ながら、用意していただいたコマンドでは動作しません。

以下のようにすれば動くと思います。

    regexp = ".*\.#{port} "
    "netstat -an -P #{protocol} 2> /dev/null | egrep 'LISTEN|Idle' | grep -- #{escape(regexp)}"

Solaris 用のコマンドが必要だけどコマンドの確認ができない場合は、
raise NotImplementedError.new してもらえると、
Solaris 用のコマンドが必要だとわかるので助かります。

@kitak

This comment has been minimized.

Show comment
Hide comment
@kitak

kitak Jul 4, 2013

Contributor

@ftnk
ありがとうございます。コマンドを修正します。
今後はraise NotImplementedError.newしますm(_ _)m

Contributor

kitak commented Jul 4, 2013

@ftnk
ありがとうございます。コマンドを修正します。
今後はraise NotImplementedError.newしますm(_ _)m

@mizzy

This comment has been minimized.

Show comment
Hide comment
@mizzy

mizzy Jul 4, 2013

これは定義しなくていいよ。method_missing で同じ事をやってくれるから。

mizzy commented on lib/serverspec/backend/exec.rb in 185a638 Jul 4, 2013

これは定義しなくていいよ。method_missing で同じ事をやってくれるから。

@mizzy

This comment has been minimized.

Show comment
Hide comment
@mizzy

mizzy Jul 4, 2013

def check_listening_with_protocol(port, protocol)
  ret = run_command(commands.check_listening_with_protocol(port, protocol))
  ret[:exit_status] == 0
end

ってすればふたつを共通にできるし、さらにこういうメソッド定義であれば、method_missing が同じ事をやってくれるから、書く必要もなくなるよね。

mizzy commented on lib/serverspec/backend/exec.rb in 185a638 Jul 4, 2013

def check_listening_with_protocol(port, protocol)
  ret = run_command(commands.check_listening_with_protocol(port, protocol))
  ret[:exit_status] == 0
end

ってすればふたつを共通にできるし、さらにこういうメソッド定義であれば、method_missing が同じ事をやってくれるから、書く必要もなくなるよね。

@mizzy

This comment has been minimized.

Show comment
Hide comment
@mizzy

mizzy Jul 4, 2013

tcp, udp 以外に増えることもなさそうだし、

if with
    backend.check_listening_with_protcol(@name, with)

とかでいいんじゃないかな。tcp か udp じゃなければ例外出す、ってのも入れとくと良さそう。

ただ、with だとちょっとわかりにくいので、

def listening?(protocol)
  if protocol 

とすると更にわかりやすくなって良さそう。

mizzy commented on lib/serverspec/type/port.rb in 185a638 Jul 4, 2013

tcp, udp 以外に増えることもなさそうだし、

if with
    backend.check_listening_with_protcol(@name, with)

とかでいいんじゃないかな。tcp か udp じゃなければ例外出す、ってのも入れとくと良さそう。

ただ、with だとちょっとわかりにくいので、

def listening?(protocol)
  if protocol 

とすると更にわかりやすくなって良さそう。

Show outdated Hide outdated spec/darwin/commands_spec.rb Outdated
Show outdated Hide outdated spec/debian/commands_spec.rb Outdated
Show outdated Hide outdated spec/gentoo/commands_spec.rb Outdated
Show outdated Hide outdated spec/redhat/commands_spec.rb Outdated
@kitak

This comment has been minimized.

Show comment
Hide comment
@kitak

kitak Jul 4, 2013

Contributor

@mizzy
ありがとうございます! 修正しました

Contributor

kitak commented Jul 4, 2013

@mizzy
ありがとうございます! 修正しました

@mizzy

This comment has been minimized.

Show comment
Hide comment
@mizzy

mizzy Jul 5, 2013

Owner

あと、be_listening.with(:tcp) じゃなく be_listening.with('tcp') にして欲しい。他のマッチャとの一貫性という意味で。

シンボルの方がオブジェクトよりもオーバーヘッド少ないだろうけど、現段階で気にすることではないし、このコードだと、文字列でもシンボルでもどちらでもいけると思うけど、あるマッチャが文字列で、別のマッチャがシンボルだと、使う人が混乱するだろうし、シンボルってRubyに慣れていない人にはわかりにくい概念なので、serverspec を利用する層の人には、わかりにくいだろうし、といった理由で、文字列で統一したいです。

それから、spec に udp のテストもあるといいな。

Owner

mizzy commented Jul 5, 2013

あと、be_listening.with(:tcp) じゃなく be_listening.with('tcp') にして欲しい。他のマッチャとの一貫性という意味で。

シンボルの方がオブジェクトよりもオーバーヘッド少ないだろうけど、現段階で気にすることではないし、このコードだと、文字列でもシンボルでもどちらでもいけると思うけど、あるマッチャが文字列で、別のマッチャがシンボルだと、使う人が混乱するだろうし、シンボルってRubyに慣れていない人にはわかりにくい概念なので、serverspec を利用する層の人には、わかりにくいだろうし、といった理由で、文字列で統一したいです。

それから、spec に udp のテストもあるといいな。

@kitak

This comment has been minimized.

Show comment
Hide comment
@kitak

kitak Jul 5, 2013

Contributor

了解です。
テストの方を文字列で渡すように修正します。

Contributor

kitak commented Jul 5, 2013

了解です。
テストの方を文字列で渡すように修正します。

@mizzy

This comment has been minimized.

Show comment
Hide comment
@mizzy

mizzy Jul 5, 2013

Owner

Good!

Owner

mizzy commented Jul 5, 2013

Good!

mizzy added a commit that referenced this pull request Jul 5, 2013

Merge pull request #200 from kitak/with_protocol
Add "be_listening.with(:protocol)" matcher for Port resource.

@mizzy mizzy merged commit 2cd6ad3 into mizzy:master Jul 5, 2013

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment