Browse files

Migrates to minitest and makes FedEx XML parsing failures more useful

  • Loading branch information...
1 parent e1ab659 commit 344cd53bec01b11b1f3291ad7b8aeb04e37cc6a4 Chris Saunders committed Aug 7, 2013
View
3 active_shipping.gemspec
@@ -22,8 +22,9 @@ Gem::Specification.new do |s|
s.add_dependency('builder')
s.add_dependency('json', '>= 1.5.1')
+ s.add_development_dependency('minitest', '~> 4.7.5')
s.add_development_dependency('rake')
- s.add_development_dependency('mocha')
+ s.add_development_dependency('mocha', '~> 0.14.0')
s.add_development_dependency('timecop')
s.add_development_dependency('nokogiri')
View
1 lib/active_shipping.rb
@@ -49,3 +49,4 @@
require 'active_shipping/shipping/shipment_packer'
require 'active_shipping/shipping/carrier'
require 'active_shipping/shipping/carriers'
+require 'active_shipping/shipping/errors'
View
12 lib/active_shipping/shipping/carriers/fedex.rb
@@ -265,7 +265,7 @@ def parse_rate_response(origin, destination, packages, response, options)
rate_estimates = []
success, message = nil
- xml = REXML::Document.new(response)
+ xml = build_document(response)
root_node = xml.elements['RateReply']
success = response_success?(xml)
@@ -330,7 +330,7 @@ def business_day?(date)
end
def parse_tracking_response(response, options)
- xml = REXML::Document.new(response)
+ xml = build_document(response)
root_node = xml.elements['TrackReply']
success = response_success?(xml)
@@ -462,6 +462,14 @@ def extract_destination(document)
Location.new(args)
end
+
+ def build_document(xml)
+ begin
+ REXML::Document.new(xml)
+ rescue REXML::ParseException => e
+ raise ActiveMerchant::Shipping::ResponseContentError.new(e, xml)
+ end
+ end
end
end
end
View
9 lib/active_shipping/shipping/errors.rb
@@ -0,0 +1,9 @@
+module ActiveMerchant
+ module Shipping
+ class ResponseContentError < StandardError
+ def initialize(exception, content_body)
+ super("#{exception.message} \n\n#{content_body}")
+ end
+ end
+ end
+end
View
27 test/fixtures/xml/fedex/invalid_fedex_reply.xml
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<v6:RateReply xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:v6="http://fedex.com/ws/rate/v6">
+ <v6:HighestSeverity>WARNING</v6:HighestSeverity>
+ <v6:Notifications>
+ <v6:Severity>WARNING</v6:Severity>
+ <v6:Source>crs</v6:Source>
+ <v6:Code>872</v6:Code>
+ <v6:Message>Rating is temporarily unavailable for one or more services:
+INTERNATIONAL_FIRST; INTERNATIONAL_PRIORITY; INTERNATIONAL_ECONOMY; ; ; ; ; ; ; ; . Please try again later. </v6:Message>
+ <v6:LocalizedMessage>Rating is temporarily unavailable for one or more services:
+INTERNATIONAL_FIRST; INTERNATIONAL_PRIORITY; INTERNATIONAL_ECONOMY; ; ; ; ; ; ; ; . Please try again later. </v6:LocalizedMessage>
+ <v6:MessageParameters>
+ <v6:Id>SERVICE_TYPE_1</v6:Id>
+ <v6:Value>INTERNATIONAL_FIRST</v6:Value>
+ </v6:MessageParameters>
+ <v6:MessageParameters>
+ <v6:Id>SERVICE_TYPE_2</v6:Id>
+ <v6:Value>INTERNATIONAL_PRIORITY</v6:Value>
+ </v6:MessageParameters>
+ <v6:MessageParameters>
+ <v6:Id>SERVICE_TYPE_3</v6:Id>
+ <v6:Value>INTERNATIONAL_ECONOMY</v6:Value>
+ </v6:MessageParameters>
+ </v6:Notifications>
+ <v6:Notifications>
+ <v6:Severity>NOTE</v6:Severity>
+ <v6:Source>crs
View
12 test/test_helper.rb
@@ -7,14 +7,14 @@
require 'test/unit'
require 'active_shipping'
-require 'mocha'
+require 'mocha/setup'
require 'timecop'
require 'nokogiri'
XmlNode # trigger autorequire
-module Test
- module Unit
+module MiniTest
+ class Unit
class TestCase
include ActiveMerchant::Shipping
@@ -69,6 +69,12 @@ def file_fixture(filename)
end
end
+module Test
+ module Unit
+ class TestCase < MiniTest::Unit::TestCase; end
+ end
+end
+
module ActiveMerchant
module Shipping
module TestFixtures
View
31 test/unit/carriers/fedex_test.rb
@@ -1,6 +1,6 @@
require 'test_helper'
-class FedExTest < Test::Unit::TestCase
+class FedExTest < MiniTest::Unit::TestCase
def setup
@packages = TestFixtures.packages
@locations = TestFixtures.locations
@@ -12,7 +12,7 @@ def test_initialize_options_requirements
assert_raises ArgumentError do FedEx.new end
assert_raises ArgumentError do FedEx.new(:login => '999999999') end
assert_raises ArgumentError do FedEx.new(:password => '7777777') end
- assert_nothing_raised { FedEx.new(:key => '999999999', :password => '7777777', :account => '123', :login => '123')}
+ FedEx.new(:key => '999999999', :password => '7777777', :account => '123', :login => '123')
end
def test_business_days
@@ -137,10 +137,8 @@ def test_find_tracking_info_should_return_shipment_events_in_ascending_chronolog
def test_find_tracking_info_should_not_include_events_without_an_address
@carrier.expects(:commit).returns(@tracking_response)
- assert_nothing_raised do
- response = @carrier.find_tracking_info('077973360403984', :test => true)
- assert_nil response.shipment_events.find{|event| event.name == 'Shipment information sent to FedEx' }
- end
+ response = @carrier.find_tracking_info('077973360403984', :test => true)
+ assert_nil response.shipment_events.find{|event| event.name == 'Shipment information sent to FedEx' }
end
def test_building_request_with_address_type_commercial_should_not_include_residential
@@ -171,7 +169,7 @@ def test_building_request_and_parsing_response
assert_instance_of Hash, response.params
assert_instance_of String, response.xml
assert_instance_of Array, response.rates
- assert_not_equal [], response.rates
+ assert response.rates.length > 0, "There should've been more than 0 rates returned"
rate = response.rates.first
assert_equal 'FedEx', rate.carrier
@@ -212,7 +210,7 @@ def test_returns_gbp_instead_of_ukl_currency_for_uk_rates
assert_equal [3836], response.rates.map(&:price)
assert response.success?, response.message
- assert_not_equal [], response.rates
+ assert response.rates.length > 0, "There should've been more than 0 rates returned"
response.rates.each do |rate|
assert_equal 'FedEx', rate.carrier
@@ -232,7 +230,7 @@ def test_returns_sgd_instead_of_sid_currency_for_signapore_rates
assert_equal [3836], response.rates.map(&:price)
assert response.success?, response.message
- assert_not_equal [], response.rates
+ assert response.rates.length > 0, "There should've been more than 0 rates returned"
response.rates.each do |rate|
assert_equal 'FedEx', rate.carrier
@@ -271,4 +269,19 @@ def test_delivery_date_from_transit_time
end
end
+ def test_failure_to_parse_invalid_xml_results_in_a_useful_error
+ mock_response = xml_fixture('fedex/invalid_fedex_reply')
+
+ @carrier.expects(:commit).returns(mock_response)
+
+ assert_raises ActiveMerchant::Shipping::ResponseContentError do
+ rate_estimates = @carrier.find_rates(
+ @locations[:ottawa],
+ @locations[:beverly_hills],
+ @packages.values_at(:book, :wii),
+ :test => true
+ )
+ end
+ end
+
end
View
8 test/unit/errors_test.rb
@@ -0,0 +1,8 @@
+require 'test_helper'
+
+class TestHelper < MiniTest::Unit::TestCase
+ def test_response_content_error_message
+ content_error = ResponseContentError.new(StandardError.new("something went wrong"), "omg omg omg")
+ assert_equal "something went wrong \n\nomg omg omg", content_error.message
+ end
+end

0 comments on commit 344cd53

Please sign in to comment.