From fe6af06027ab7d2fdbcc73bcf745fd6253e470ba Mon Sep 17 00:00:00 2001 From: Yasuhito Takamiya Date: Wed, 8 Jul 2015 10:33:45 +0900 Subject: [PATCH] Refactor Pio::Hello and its tests. --- Rakefile | 2 +- features/open_flow10/echo_reply.feature | 16 +---- features/open_flow10/echo_request.feature | 14 ----- features/open_flow10/hello.feature | 20 +++---- features/open_flow13/echo_reply.feature | 2 +- features/open_flow13/echo_request.feature | 2 +- features/open_flow13/features_request.feature | 2 +- lib/pio/open_flow/echo.rb | 11 ++-- lib/pio/open_flow/message.rb | 20 +++++++ lib/pio/open_flow10/hello.rb | 12 +++- lib/pio/open_flow13/features_request.rb | 2 +- lib/pio/open_flow13/hello.rb | 25 ++++---- spec/pio/open_flow10/echo_reply_spec.rb | 7 +++ spec/pio/open_flow10/echo_request_spec.rb | 7 +++ spec/pio/open_flow10/hello_spec.rb | 7 +++ spec/pio/open_flow13/hello_spec.rb | 59 ++----------------- .../shared_examples_for_openflow_messages.rb | 38 ++++++++++++ 17 files changed, 129 insertions(+), 117 deletions(-) create mode 100644 spec/pio/open_flow10/echo_reply_spec.rb create mode 100644 spec/pio/open_flow10/echo_request_spec.rb create mode 100644 spec/pio/open_flow10/hello_spec.rb diff --git a/Rakefile b/Rakefile index 8daeb4a3..86761b78 100644 --- a/Rakefile +++ b/Rakefile @@ -1,7 +1,7 @@ require 'bundler/gem_tasks' RELISH_PROJECT = 'trema/pio' -FLAY_THRESHOLD = 1539 +FLAY_THRESHOLD = 1467 task default: :travis task test: [:spec, :cucumber] diff --git a/features/open_flow10/echo_reply.feature b/features/open_flow10/echo_reply.feature index 29e02c1c..50e7c2fc 100644 --- a/features/open_flow10/echo_reply.feature +++ b/features/open_flow10/echo_reply.feature @@ -50,26 +50,12 @@ Feature: Pio::Echo::Reply | body | | | user_data | | - Scenario: new(xid: -1) and error - When I try to create an OpenFlow message with: - """ - Pio::Echo::Reply.new(xid: -1) - """ - Then it should fail with "ArgumentError", "Transaction ID should be an unsigned 32-bit integer." - - Scenario: new(xid: 2**32) and error - When I try to create an OpenFlow message with: - """ - Pio::Echo::Reply.new(xid: 2**32) - """ - Then it should fail with "ArgumentError", "Transaction ID should be an unsigned 32-bit integer." - Scenario: new(unknown_attr: 'foo') and error When I try to create an OpenFlow message with: """ Pio::Echo::Reply.new(unknown_attr: 'foo') """ - Then it should fail with "RuntimeError", "Unknown keyword: unknown_attr" + Then it should fail with "RuntimeError", "Unknown option: unknown_attr" Scenario: new(body: 'echo reply body') When I try to create an OpenFlow message with: diff --git a/features/open_flow10/echo_request.feature b/features/open_flow10/echo_request.feature index 414758db..242777d2 100644 --- a/features/open_flow10/echo_request.feature +++ b/features/open_flow10/echo_request.feature @@ -50,20 +50,6 @@ Feature: Pio::Echo::Request | body | | | user_data | | - Scenario: new(xid: -1) and error - When I try to create an OpenFlow message with: - """ - Pio::Echo::Request.new(xid: -1) - """ - Then it should fail with "ArgumentError", "Transaction ID should be an unsigned 32-bit integer." - - Scenario: new(xid: 2**32) and error - When I try to create an OpenFlow message with: - """ - Pio::Echo::Request.new(xid: 2**32) - """ - Then it should fail with "ArgumentError", "Transaction ID should be an unsigned 32-bit integer." - Scenario: new(body: 'echo request body') When I try to create an OpenFlow message with: """ diff --git a/features/open_flow10/hello.feature b/features/open_flow10/hello.feature index 33cbd229..41441f78 100644 --- a/features/open_flow10/hello.feature +++ b/features/open_flow10/hello.feature @@ -14,6 +14,7 @@ Feature: Pio::Hello | transaction_id | 0 | | xid | 0 | | body | | + | user_data | | Scenario: new(transaction_id: 123) When I try to create an OpenFlow message with: @@ -30,7 +31,8 @@ Feature: Pio::Hello | transaction_id | 123 | | xid | 123 | | body | | - + | user_data | | + Scenario: new(xid: 123) When I try to create an OpenFlow message with: """ @@ -46,20 +48,14 @@ Feature: Pio::Hello | transaction_id | 123 | | xid | 123 | | body | | + | user_data | | - Scenario: new(xid: -1) and error - When I try to create an OpenFlow message with: - """ - Pio::Hello.new(xid: -1) - """ - Then it should fail with "ArgumentError", "Transaction ID should be an unsigned 32-bit integer." - - Scenario: new(xid: 2**32) and error + Scenario: new(unknown_attr: 'foo') and error When I try to create an OpenFlow message with: """ - Pio::Hello.new(xid: 2**32) + Pio::Hello.new(unknown_attr: 'foo') """ - Then it should fail with "ArgumentError", "Transaction ID should be an unsigned 32-bit integer." + Then it should fail with "RuntimeError", "Unknown option: unknown_attr" Scenario: new(body: 'hello') When I try to create an OpenFlow message with: @@ -76,6 +72,7 @@ Feature: Pio::Hello | transaction_id | 0 | | xid | 0 | | body | hello | + | user_data | hello | Scenario: read When I try to parse a file named "open_flow10/hello.raw" with "Hello" class @@ -89,6 +86,7 @@ Feature: Pio::Hello | transaction_id | 23 | | xid | 23 | | body | | + | user_data | | Scenario: parse error When I try to parse a file named "open_flow10/features_request.raw" with "Pio::Hello" class diff --git a/features/open_flow13/echo_reply.feature b/features/open_flow13/echo_reply.feature index 8e1548d0..37f83012 100644 --- a/features/open_flow13/echo_reply.feature +++ b/features/open_flow13/echo_reply.feature @@ -106,7 +106,7 @@ Feature: Pio::Echo::Reply """ Pio::Echo::Reply.new(unknown_attr: 'foo') """ - Then it should fail with "RuntimeError", "Unknown keyword: unknown_attr" + Then it should fail with "RuntimeError", "Unknown option: unknown_attr" Scenario: read (no message body) When I try to parse a file named "open_flow13/echo_reply_no_body.raw" with "Pio::Echo::Reply" class diff --git a/features/open_flow13/echo_request.feature b/features/open_flow13/echo_request.feature index 1b1f5e04..04cbd97a 100644 --- a/features/open_flow13/echo_request.feature +++ b/features/open_flow13/echo_request.feature @@ -106,7 +106,7 @@ Feature: Pio::Echo::Request """ Pio::Echo::Request.new(unknown_attr: 'foo') """ - Then it should fail with "RuntimeError", "Unknown keyword: unknown_attr" + Then it should fail with "RuntimeError", "Unknown option: unknown_attr" Scenario: read (no message body) When I try to parse a file named "open_flow13/echo_request_no_body.raw" with "Pio::Echo::Request" class diff --git a/features/open_flow13/features_request.feature b/features/open_flow13/features_request.feature index dd495e16..69a4eb74 100644 --- a/features/open_flow13/features_request.feature +++ b/features/open_flow13/features_request.feature @@ -69,7 +69,7 @@ Feature: Pio::Features::Request """ Pio::Features::Request.new(unknown_attr: 'foo') """ - Then it should fail with "RuntimeError", "Unknown keyword: unknown_attr" + Then it should fail with "RuntimeError", "Unknown option: unknown_attr" Scenario: read When I try to parse a file named "open_flow13/features_request.raw" with "Pio::Features::Request" class diff --git a/lib/pio/open_flow/echo.rb b/lib/pio/open_flow/echo.rb index d1fe79a8..bfd67668 100644 --- a/lib/pio/open_flow/echo.rb +++ b/lib/pio/open_flow/echo.rb @@ -36,12 +36,13 @@ def self.message_type(message_type) end end + user_option :transaction_id + user_option :xid + user_option :body + user_option :user_data + def initialize(user_options = {}) - unknown_options = - user_options.keys - [:transaction_id, :xid, :body, :user_data] - unless unknown_options.empty? - fail "Unknown keyword: #{unknown_options.first}" - end + validate_user_options user_options header_options = OpenFlowHeader::Options.parse(user_options) body_options = user_options[:body] || user_options[:user_data] || '' @format = self.class.const_get(:Format).new(header: header_options, diff --git a/lib/pio/open_flow/message.rb b/lib/pio/open_flow/message.rb index 94092654..af466ca3 100644 --- a/lib/pio/open_flow/message.rb +++ b/lib/pio/open_flow/message.rb @@ -19,9 +19,29 @@ def self.read(raw_data) end # rubocop:enable MethodLength + def self.user_option(name) + unless class_variable_defined?(:@@valid_options) + class_variable_set(:@@valid_options, []) + end + class_variable_set(:@@valid_options, + class_variable_get(:@@valid_options) + [name]) + end + def method_missing(method, *args, &block) @format.__send__ method, *args, &block end + + private + + def validate_user_options(user_options) + unknown_options = user_options.keys - valid_options + return if unknown_options.empty? + fail "Unknown option: #{unknown_options.first}" + end + + def valid_options + self.class.class_variable_get(:@@valid_options) + end end end end diff --git a/lib/pio/open_flow10/hello.rb b/lib/pio/open_flow10/hello.rb index 94b0fd7b..5f849096 100644 --- a/lib/pio/open_flow10/hello.rb +++ b/lib/pio/open_flow10/hello.rb @@ -4,15 +4,25 @@ module Pio # OpenFlow 1.0 Hello message class Hello < OpenFlow::Message - # OpenFlow 1.0 Hello message + # OpenFlow 1.0 Hello message format class Format < BinData::Record extend OpenFlow::Format header version: 1, message_type: OpenFlow::HELLO string :body + + def user_data + body + end end + user_option :transaction_id + user_option :xid + user_option :body + user_option :user_data + def initialize(user_options = {}) + validate_user_options user_options header_options = OpenFlowHeader::Options.parse(user_options) body_options = user_options[:body] || user_options[:user_data] || '' @format = Format.new(header: header_options, body: body_options) diff --git a/lib/pio/open_flow13/features_request.rb b/lib/pio/open_flow13/features_request.rb index 93829773..2ee0f358 100644 --- a/lib/pio/open_flow13/features_request.rb +++ b/lib/pio/open_flow13/features_request.rb @@ -19,7 +19,7 @@ class Format < BinData::Record def initialize(user_attrs = {}) unknown_attrs = user_attrs.keys - [:transaction_id, :xid] unless unknown_attrs.empty? - fail "Unknown keyword: #{unknown_attrs.first}" + fail "Unknown option: #{unknown_attrs.first}" end header_options = OpenFlowHeader::Options.parse(user_attrs) @format = Format.new(header: header_options) diff --git a/lib/pio/open_flow13/hello.rb b/lib/pio/open_flow13/hello.rb index a5f563ca..63360192 100644 --- a/lib/pio/open_flow13/hello.rb +++ b/lib/pio/open_flow13/hello.rb @@ -2,14 +2,14 @@ # Base module. module Pio - remove_const :Hello + remove_const :Hello if const_defined?(:Hello) # OpenFlow 1.3 Hello message parser and generator class Hello < OpenFlow::Message + VERSION_BITMAP = 1 + # ofp_hello_elem_header and value class Element < BinData::Record - VERSION_BITMAP = 1 - endian :big uint16 :element_type @@ -70,17 +70,16 @@ def version_bitmap end end - def initialize(user_attrs = {}) - unknown_keywords = user_attrs.keys - [:transaction_id, :xid] - unless unknown_keywords.empty? - fail "Unknown keyword: #{unknown_keywords.first}" - end + user_option :transaction_id + user_option :xid - header_attrs = OpenFlowHeader::Options.parse(user_attrs) - body_attrs = { elements: [{ element_type: 1, - element_length: 8, - element_value: 16 }] } - @format = Format.new(header: header_attrs, body: body_attrs) + def initialize(user_options = {}) + validate_user_options user_options + header_options = OpenFlowHeader::Options.parse(user_options) + body_options = { elements: [{ element_type: VERSION_BITMAP, + element_length: 8, + element_value: 0b10000 }] } + @format = Format.new(header: header_options, body: body_options) end end end diff --git a/spec/pio/open_flow10/echo_reply_spec.rb b/spec/pio/open_flow10/echo_reply_spec.rb new file mode 100644 index 00000000..bfe3cdb5 --- /dev/null +++ b/spec/pio/open_flow10/echo_reply_spec.rb @@ -0,0 +1,7 @@ +require 'pio/open_flow10/echo' + +describe Pio::Echo::Reply do + describe '.new' do + it_should_behave_like('an OpenFlow message', Pio::Echo::Reply) + end +end diff --git a/spec/pio/open_flow10/echo_request_spec.rb b/spec/pio/open_flow10/echo_request_spec.rb new file mode 100644 index 00000000..747e4f59 --- /dev/null +++ b/spec/pio/open_flow10/echo_request_spec.rb @@ -0,0 +1,7 @@ +require 'pio/open_flow10/echo' + +describe Pio::Echo::Request do + describe '.new' do + it_should_behave_like('an OpenFlow message', Pio::Echo::Request) + end +end diff --git a/spec/pio/open_flow10/hello_spec.rb b/spec/pio/open_flow10/hello_spec.rb new file mode 100644 index 00000000..bab1b32c --- /dev/null +++ b/spec/pio/open_flow10/hello_spec.rb @@ -0,0 +1,7 @@ +require 'pio/open_flow10/hello' + +describe Pio::Hello do + describe '.new' do + it_should_behave_like('an OpenFlow message', Pio::Hello) + end +end diff --git a/spec/pio/open_flow13/hello_spec.rb b/spec/pio/open_flow13/hello_spec.rb index 2cfcf887..65d7d1fb 100644 --- a/spec/pio/open_flow13/hello_spec.rb +++ b/spec/pio/open_flow13/hello_spec.rb @@ -1,4 +1,5 @@ require 'pio/open_flow13/hello' +require 'pio/parse_error' describe Pio::Hello do describe '.read' do @@ -38,6 +39,8 @@ end describe '.new' do + it_should_behave_like('an OpenFlow message', Pio::Hello) + context 'with no arguments' do When(:result) { Pio::Hello.new } @@ -53,61 +56,11 @@ end end - context 'with transaction_id: 123' do - When(:result) { Pio::Hello.new(transaction_id: 123) } - - Then { result.ofp_version == 4 } - Then { result.message_type == 0 } - Then { result.message_length == 16 } - Then { result.transaction_id == 123 } - Then { result.xid == 123 } - Then { result.supported_versions == [:open_flow13] } - Then do - result.to_binary == - [4, 0, 0, 16, 0, 0, 0, 123, 0, 1, 0, 8, 0, 0, 0, 16].pack('C*') - end - end - - context 'with xid: 123' do - When(:result) { Pio::Hello.new(xid: 123) } - - Then { result.ofp_version == 4 } - Then { result.message_type == 0 } - Then { result.message_length == 16 } - Then { result.transaction_id == 123 } - Then { result.xid == 123 } - Then { result.supported_versions == [:open_flow13] } - Then do - result.to_binary == - [4, 0, 0, 16, 0, 0, 0, 123, 0, 1, 0, 8, 0, 0, 0, 16].pack('C*') - end - end - - context 'with xid: -1' do - When(:result) { Pio::Hello.new(xid: -1) } - - Then do - result == - Failure(ArgumentError, - 'Transaction ID should be an unsigned 32-bit integer.') - end - end - - context 'with xid: 2**32' do - When(:result) { Pio::Hello.new(xid: 2**32) } - - Then do - result == - Failure(ArgumentError, - 'Transaction ID should be an unsigned 32-bit integer.') - end - end - - context 'with invalid_keyword: :foo' do - When(:result) { Pio::Hello.new(invalid_keyword: :foo) } + context 'with invalid_option: :foo' do + When(:result) { Pio::Hello.new(invalid_option: :foo) } Then do - result == Failure(RuntimeError, 'Unknown keyword: invalid_keyword') + result == Failure(RuntimeError, 'Unknown option: invalid_option') end end end diff --git a/spec/support/shared_examples_for_openflow_messages.rb b/spec/support/shared_examples_for_openflow_messages.rb index 16dc2a0f..fb5a2a47 100644 --- a/spec/support/shared_examples_for_openflow_messages.rb +++ b/spec/support/shared_examples_for_openflow_messages.rb @@ -1,3 +1,41 @@ +shared_examples 'an OpenFlow message' do |klass| + When(:message) { klass.new(options) } + + context 'with transaction_id: 123' do + Given(:options) { { transaction_id: 123 } } + + Then { message.transaction_id == 123 } + Then { message.xid == 123 } + end + + context 'with xid: 123' do + When(:options) { { xid: 123 } } + + Then { message.transaction_id == 123 } + Then { message.xid == 123 } + end + + context 'with xid: -1' do + When(:options) { { xid: -1 } } + + Then do + message == + Failure(ArgumentError, + 'Transaction ID should be an unsigned 32-bit integer.') + end + end + + context 'with xid: 2**32' do + When(:options) { { xid: 2**32 } } + + Then do + message == + Failure(ArgumentError, + 'Transaction ID should be an unsigned 32-bit integer.') + end + end +end + shared_examples 'an OpenFlow message with Datapath ID' do |klass| When(:message) { klass.new(options) }