Skip to content

Commit

Permalink
Improvements to the reporting of invalid identifiers.
Browse files Browse the repository at this point in the history 
- Include the identifier in all error cases in
  ZoneinfoDataSource#load_timezone_info.
- Include the country code in InvalidCountryCode messages raised in
  ZoneinfoDataSource#load_country_info (this was already being done in
  the RubyDatasSource equivalent).
- Minor changes to the format of the exception messages.
- Add additional test cases.
- Make tests match on word boundaries to avoid matching filenames, etc
  that may also be included in the message.
  • Loading branch information
@philr
philr committed May 21, 2016
1 parent 36c80e1 commit 87fc615
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 63 deletions.
6 changes: 3 additions & 3 deletions lib/tzinfo/ruby_data_source.rb
View file
Expand Up @@ -20,7 +20,7 @@ class RubyDataSource < DataSource
# identifier is invalid.
def load_timezone_info(identifier)
original_identifier = identifier
raise InvalidTimezoneIdentifier, "Invalid identifier - #{original_identifier}" if identifier !~ /^[A-Za-z0-9\+\-_]+(\/[A-Za-z0-9\+\-_]+)*$/
raise InvalidTimezoneIdentifier, "Invalid identifier: #{original_identifier}" if identifier !~ /^[A-Za-z0-9\+\-_]+(\/[A-Za-z0-9\+\-_]+)*$/

identifier = identifier.gsub(/-/, '__m__').gsub(/\+/, '__p__')

Expand All @@ -40,7 +40,7 @@ def load_timezone_info(identifier)

m.get
rescue LoadError, NameError => e
raise InvalidTimezoneIdentifier, "#{e.message} - #{original_identifier}"
raise InvalidTimezoneIdentifier, "#{e.message} (loading #{original_identifier})"
end
end

Expand Down Expand Up @@ -70,7 +70,7 @@ def linked_timezone_identifiers
def load_country_info(code)
load_country_index
info = Data::Indexes::Countries.countries[code]
raise InvalidCountryCode, "Invalid country code - #{code}" unless info
raise InvalidCountryCode, "Invalid country code: #{code}" unless info
info
end

Expand Down
10 changes: 5 additions & 5 deletions lib/tzinfo/zoneinfo_data_source.rb
View file
Expand Up @@ -202,15 +202,15 @@ def load_timezone_info(identifier)
begin
ZoneinfoTimezoneInfo.new(identifier, path)
rescue InvalidZoneinfoFile => e
raise InvalidTimezoneIdentifier, e.message
raise InvalidTimezoneIdentifier, "#{e.message} (loading #{identifier})"
end
else
raise InvalidTimezoneIdentifier, "Invalid identifier - #{identifier}"
raise InvalidTimezoneIdentifier, "Invalid identifier: #{identifier}"
end
rescue Errno::ENOENT, Errno::ENAMETOOLONG, Errno::ENOTDIR
raise InvalidTimezoneIdentifier, "Invalid identifier - #{identifier}"
raise InvalidTimezoneIdentifier, "Invalid identifier: #{identifier}"
rescue Errno::EACCES => e
raise InvalidTimezoneIdentifier, e.message
raise InvalidTimezoneIdentifier, "#{e.message} (loading #{identifier})"
end
end

Expand Down Expand Up @@ -241,7 +241,7 @@ def linked_timezone_identifiers
# or the code is invalid.
def load_country_info(code)
info = @country_index[code]
raise InvalidCountryCode, 'Invalid country code' unless info
raise InvalidCountryCode, "Invalid country code: #{code}" unless info
info
end

Expand Down
28 changes: 10 additions & 18 deletions test/tc_country.rb
View file
Expand Up @@ -20,27 +20,22 @@ def test_get_valid
end

def test_get_not_exist
assert_raises(InvalidCountryCode) {
Country.get('ZZ')
}
error = assert_raises(InvalidCountryCode) { Country.get('ZZ') }
assert_match(/\bZZ\b/, error.message)
end

def test_get_invalid
assert_raises(InvalidCountryCode) {
Country.get('../Countries/GB')
}
error = assert_raises(InvalidCountryCode) { Country.get('../Countries/GB') }
assert_match(/\W\.\.\/Countries\/GB\b/, error.message)
end

def test_get_nil
assert_raises(InvalidCountryCode) {
Country.get(nil)
}
assert_raises(InvalidCountryCode) { Country.get(nil) }
end

def test_get_case
assert_raises(InvalidCountryCode) {
Country.get('gb')
}
error = assert_raises(InvalidCountryCode) { Country.get('gb') }
assert_match(/\bgb\b/, error.message)
end

def test_get_tainted_loaded
Expand Down Expand Up @@ -82,9 +77,7 @@ def test_get_tainted_and_frozen_not_previously_loaded
end

def test_new_nil
assert_raises(InvalidCountryCode) {
Country.new(nil)
}
assert_raises(InvalidCountryCode) { Country.new(nil) }
end

def test_new_arg
Expand All @@ -93,9 +86,8 @@ def test_new_arg
end

def test_new_arg_not_exist
assert_raises(InvalidCountryCode) {
Country.new('ZZ')
}
error = assert_raises(InvalidCountryCode) { Country.new('ZZ') }
assert_match(/\bZZ\b/, error.message)
end

def test_all_codes
Expand Down
18 changes: 12 additions & 6 deletions test/tc_ruby_data_source.rb
View file
Expand Up @@ -25,13 +25,15 @@ def test_load_timezone_info_does_not_exist
@data_source.load_timezone_info('Nowhere/Special')
end

assert_match 'Nowhere/Special', error.message
assert_match(/\bNowhere\/Special\b/, error.message)
end

def test_load_timezone_info_invalid
assert_raises(InvalidTimezoneIdentifier) do
error = assert_raises(InvalidTimezoneIdentifier) do
@data_source.load_timezone_info('../Definitions/UTC')
end

assert_match(/\W\.\.\/Definitions\/UTC\b/, error.message)
end

def test_load_timezone_info_nil
Expand All @@ -45,7 +47,7 @@ def test_load_timezone_info_case
@data_source.load_timezone_info('europe/london')
end

assert_match 'europe/london', error.message
assert_match(/\beurope\/london\b/, error.message)
end

def test_load_timezone_info_plus
Expand Down Expand Up @@ -103,13 +105,15 @@ def test_load_country_info_not_exist
@data_source.load_country_info('ZZ')
end

assert_match 'ZZ', error.message
assert_match(/\bZZ\b/, error.message)
end

def test_load_country_info_invalid
assert_raises(InvalidCountryCode) do
error = assert_raises(InvalidCountryCode) do
@data_source.load_country_info('../Countries/GB')
end

assert_match(/\W\.\.\/Countries\/GB\b/, error.message)
end

def test_load_country_info_nil
Expand All @@ -119,9 +123,11 @@ def test_load_country_info_nil
end

def test_load_country_info_case
assert_raises(InvalidCountryCode) do
error = assert_raises(InvalidCountryCode) do
@data_source.load_country_info('gb')
end

assert_match(/\bgb\b/, error.message)
end

def test_load_country_info_tainted
Expand Down
16 changes: 7 additions & 9 deletions test/tc_timezone.rb
View file
Expand Up @@ -205,15 +205,13 @@ def test_get_same_instance
end

def test_get_not_exist
error = assert_raises(InvalidTimezoneIdentifier) do
Timezone.get('Nowhere/Special')
end

assert_match 'Nowhere/Special', error.message
error = assert_raises(InvalidTimezoneIdentifier) { Timezone.get('Nowhere/Special') }
assert_match(/\bNowhere\/Special/, error.message)
end

def test_get_invalid
assert_raises(InvalidTimezoneIdentifier) { Timezone.get('../Definitions/UTC') }
error = assert_raises(InvalidTimezoneIdentifier) { Timezone.get('../Definitions/UTC') }
assert_match(/\W\.\.\/Definitions\/UTC\b/, error.message)
end

def test_get_nil
Expand All @@ -223,8 +221,7 @@ def test_get_nil
def test_get_case
Timezone.get('Europe/Prague')
error = assert_raises(InvalidTimezoneIdentifier) { Timezone.get('Europe/prague') }

assert_match 'Europe/prague', error.message
assert_match(/\bEurope\/prague\b/, error.message)
end

def test_get_proxy_valid
Expand Down Expand Up @@ -323,7 +320,8 @@ def test_new_arg
end

def test_new_arg_not_exist
assert_raises(InvalidTimezoneIdentifier) { Timezone.new('Nowhere/Special') }
error = assert_raises(InvalidTimezoneIdentifier) { Timezone.new('Nowhere/Special') }
assert_match(/\bNowhere\/Special\b/, error.message)
end

def test_all
Expand Down
25 changes: 15 additions & 10 deletions test/tc_timezone_proxy.rb
View file
Expand Up @@ -3,19 +3,24 @@
include TZInfo

class TCTimezoneProxy < Minitest::Test
def assert_raises_invalid_timezone_identifier(identifier)
error = assert_raises(InvalidTimezoneIdentifier) { yield }
assert_match(Regexp.new('\b' + Regexp.escape(identifier) + '\b'), error.message)
end

def test_not_exist
proxy = TimezoneProxy.new('Nothing/Special')
assert_equal('Nothing/Special', proxy.identifier)
assert_raises(InvalidTimezoneIdentifier) { proxy.now }
assert_raises(InvalidTimezoneIdentifier) { proxy.current_period }
assert_raises(InvalidTimezoneIdentifier) { proxy.current_period_and_time }
assert_raises(InvalidTimezoneIdentifier) { proxy.current_time_and_period }
assert_raises(InvalidTimezoneIdentifier) { proxy.utc_to_local(DateTime.new(2006,1,1,0,0,0)) }
assert_raises(InvalidTimezoneIdentifier) { proxy.local_to_utc(DateTime.new(2006,1,1,0,0,0)) }
assert_raises(InvalidTimezoneIdentifier) { proxy.period_for_utc(DateTime.new(2006,1,1,0,0,0)) }
assert_raises(InvalidTimezoneIdentifier) { proxy.period_for_local(DateTime.new(2006,1,1,0,0,0)) }
assert_raises(InvalidTimezoneIdentifier) { proxy.canonical_identifier }
assert_raises(InvalidTimezoneIdentifier) { proxy.canonical_zone }
assert_raises_invalid_timezone_identifier('Nothing/Special') { proxy.now }
assert_raises_invalid_timezone_identifier('Nothing/Special') { proxy.current_period }
assert_raises_invalid_timezone_identifier('Nothing/Special') { proxy.current_period_and_time }
assert_raises_invalid_timezone_identifier('Nothing/Special') { proxy.current_time_and_period }
assert_raises_invalid_timezone_identifier('Nothing/Special') { proxy.utc_to_local(DateTime.new(2006,1,1,0,0,0)) }
assert_raises_invalid_timezone_identifier('Nothing/Special') { proxy.local_to_utc(DateTime.new(2006,1,1,0,0,0)) }
assert_raises_invalid_timezone_identifier('Nothing/Special') { proxy.period_for_utc(DateTime.new(2006,1,1,0,0,0)) }
assert_raises_invalid_timezone_identifier('Nothing/Special') { proxy.period_for_local(DateTime.new(2006,1,1,0,0,0)) }
assert_raises_invalid_timezone_identifier('Nothing/Special') { proxy.canonical_identifier }
assert_raises_invalid_timezone_identifier('Nothing/Special') { proxy.canonical_zone }
end

def test_valid
Expand Down
38 changes: 26 additions & 12 deletions test/tc_zoneinfo_data_source.rb
View file
Expand Up @@ -362,21 +362,23 @@ def test_load_timezone_info_does_not_exist
@data_source.load_timezone_info('Nowhere/Special')
end

assert_match 'Nowhere/Special', error.message
assert_match(/\bNowhere\/Special\b/, error.message)
end

def test_load_timezone_info_invalid
assert_raises(InvalidTimezoneIdentifier) do
error = assert_raises(InvalidTimezoneIdentifier) do
@data_source.load_timezone_info('../Definitions/Europe/London')
end

assert_match(/\W\.\.\/Definitions\/Europe\/London\b/, error.message)
end

def test_load_timezone_info_ignored_file
error = assert_raises(InvalidTimezoneIdentifier) do
@data_source.load_timezone_info('localtime')
end

assert_match 'localtime', error.message
assert_match(/\blocaltime\b/, error.message)
end

def test_load_timezone_info_ignored_plus_version_file
Expand All @@ -393,9 +395,11 @@ def test_load_timezone_info_ignored_plus_version_file

data_source = ZoneinfoDataSource.new(dir)

assert_raises(InvalidTimezoneIdentifier) do
error = assert_raises(InvalidTimezoneIdentifier) do
data_source.load_timezone_info('+VERSION')
end

assert_match(/\W\+VERSION/, error.message)
end
end

Expand All @@ -410,7 +414,7 @@ def test_load_timezone_info_case
@data_source.load_timezone_info('europe/london')
end

assert_match 'europe/london', error.message
assert_match(/\beurope\/london/, error.message)
end

def test_load_timezone_info_permission_denied
Expand All @@ -424,9 +428,11 @@ def test_load_timezone_info_permission_denied

data_source = ZoneinfoDataSource.new(dir)

assert_raises(InvalidTimezoneIdentifier) do
error = assert_raises(InvalidTimezoneIdentifier) do
data_source.load_timezone_info('UTC')
end

assert_match(/\bUTC\b/, error.message)
end
end

Expand All @@ -440,9 +446,11 @@ def test_load_timezone_info_directory

data_source = ZoneinfoDataSource.new(dir)

assert_raises(InvalidTimezoneIdentifier) do
error = assert_raises(InvalidTimezoneIdentifier) do
data_source.load_timezone_info('Subdir')
end

assert_match(/\bSubdir\b/, error.message)
end
end

Expand Down Expand Up @@ -591,7 +599,7 @@ def test_load_timezone_info_invalid_file
data_source.load_timezone_info('Zone')
end

assert_match 'Zone', error.message
assert_match(/\bZone\b/, error.message)
end
end

Expand All @@ -617,7 +625,7 @@ def test_load_timezone_info_invalid_file_2
data_source.load_timezone_info('Zone')
end

assert_match 'Zone', error.message
assert_match(/\bZone\b/, error.message)
end
end

Expand Down Expand Up @@ -753,15 +761,19 @@ def test_load_country_info
end

def test_load_country_info_not_exist
assert_raises(InvalidCountryCode) do
error = assert_raises(InvalidCountryCode) do
@data_source.load_country_info('ZZ')
end

assert_match(/\bZZ\b/, error.message)
end

def test_load_country_info_invalid
assert_raises(InvalidCountryCode) do
error = assert_raises(InvalidCountryCode) do
@data_source.load_country_info('../Countries/GB')
end

assert_match(/\W\.\.\/Countries\/GB\b/, error.message)
end

def test_load_country_info_nil
Expand All @@ -771,9 +783,11 @@ def test_load_country_info_nil
end

def test_load_country_info_case
assert_raises(InvalidCountryCode) do
error = assert_raises(InvalidCountryCode) do
@data_source.load_country_info('gb')
end

assert_match(/\bgb\b/, error.message)
end

def test_load_country_info_tainted
Expand Down

0 comments on commit 87fc615

Please sign in to comment.