Fix bug on a test, improves code style

Signed-off-by: Jorge J. Perez <jjperez@ekumenlabs.com>

ament_index_python/test/test_ament_index_python.py

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 import os
-
+from pathlib import Path, PurePath
 
 from ament_index_python import get_package_prefix
 from ament_index_python import get_package_share_directory
@@ -23,32 +23,31 @@
 from ament_index_python import get_resources
 from ament_index_python import get_search_paths
 from ament_index_python import has_resource
-from ament_index_python import main
 from ament_index_python import PackageNotFoundError
-from ament_index_python import resource_name_completer
-from ament_index_python import resource_type_completer
+from ament_index_python.cli import main
+from ament_index_python.cli import resource_name_completer
+from ament_index_python.cli import resource_type_completer
+
+import pytest
 
 
 def set_ament_prefix_path(subfolders):
     paths = []
-    base_path = os.path.dirname(__file__)
+    base_path = Path(__file__).parents[0]
     for subfolder in subfolders:
-        path = os.path.join(base_path, subfolder)
-        if os.path.isdir(path):
-            paths.append(path)
+        path = base_path / subfolder
+        if path.is_dir():
+            paths.append(str(path))
     ament_prefix_path = os.pathsep.join(paths)
     os.environ['AMENT_PREFIX_PATH'] = ament_prefix_path
 
 
 def test_empty_search_paths():
     set_ament_prefix_path([])
-    try:
+    with pytest.raises(EnvironmentError) as excinfo:
         get_search_paths()
-        assert False, 'get_search_paths() failed to raise exception'
-    except EnvironmentError:
-        pass
-    except Exception as e:
-        assert False, 'get_search_paths() raised wrong exception: ' + type(e)
+    assert excinfo.type is EnvironmentError, (
+        f'Expected EnvironmentError, got: {excinfo.type}')
 
 
 def test_search_paths():
@@ -73,8 +72,7 @@ def test_resources():
     set_ament_prefix_path(['prefix1'])
     resources = get_resources('resource_type1')
     assert len(resources) == 2, 'Expected two resources'
-    assert set(resources.keys()) == {'foo', 'bar'}, \
-        'Expected different resources'
+    assert set(resources.keys()) == {'foo', 'bar'}, 'Expected different resources'
 
 
 def test_resources_overlay():
@@ -97,13 +95,10 @@ def test_unknown_resource():
     exists = has_resource('resource_type4', 'bar')
     assert not exists, 'Resource should not exist'
 
-    try:
+    with pytest.raises(LookupError) as excinfo:
         get_resource('resource_type4', 'bar')
-        assert False, 'get_resource() failed to raise exception'
-    except LookupError:
-        pass
-    except Exception as e:
-        assert False, 'get_resource() raised wrong exception: ' + type(e)
+    assert excinfo.type is LookupError, (
+        f'Expected LookupError, got: {excinfo.type}')
 
 
 def test_resource():
@@ -113,30 +108,27 @@ def test_resource():
 
     resource, prefix = get_resource('resource_type4', 'foo')
     assert resource == 'foo', 'Expected different content'
-    assert os.path.basename(prefix) == 'prefix1', 'Expected different prefix'
+    assert PurePath(prefix).name == 'prefix1', 'Expected different prefix'
 
 
 def test_resource_overlay():
     set_ament_prefix_path(['prefix1', 'prefix2'])
 
     resource, prefix = get_resource('resource_type5', 'foo')
     assert resource == 'foo1', 'Expected different content'
-    assert os.path.basename(prefix) == 'prefix1', 'Expected different prefix'
+    assert PurePath(prefix).name == 'prefix1', 'Expected different prefix'
 
 
 def test_get_packages_with_prefixes():
     set_ament_prefix_path(['prefix1', 'prefix2'])
 
     packages = get_packages_with_prefixes()
     assert 'foo' in packages, "Expected to find 'foo'"
-    assert os.path.basename(packages['foo']) == 'prefix1', \
-        "Expected to find 'foo' in 'prefix1'"
+    assert PurePath(packages['foo']).name == 'prefix1', "Expected to find 'foo' in 'prefix1'"
     assert 'bar' in packages, "Expected to find 'bar'"
-    assert os.path.basename(packages['bar']) == 'prefix1', \
-        "Expected to find 'bar' in 'prefix1'"
+    assert PurePath(packages['bar']).name == 'prefix1', "Expected to find 'bar' in 'prefix1'"
     assert 'baz' in packages, "Expected to find 'baz'"
-    assert os.path.basename(packages['baz']) == 'prefix2', \
-        "Expected to find 'baz' in 'prefix2'"
+    assert PurePath(packages['baz']).name == 'prefix2', "Expected to find 'baz' in 'prefix2'"
     os.environ['AMENT_PREFIX_PATH'] = '/path/does/not/exist'
 
     assert not get_packages_with_prefixes(), 'Expected to find no packages'
@@ -146,31 +138,16 @@ def test_get_package_prefix():
     set_ament_prefix_path(['prefix1', 'prefix2'])
 
     def get_package_prefix_basename(package_name):
-        return os.path.basename(get_package_prefix(package_name))
+        return PurePath(get_package_prefix(package_name)).name
 
-    assert get_package_prefix_basename('foo') == 'prefix1', \
-        "Expected 'foo' in 'prefix1'"
-    # found in both prefix1 and prefix2, but prefix1 is ahead on the APP
-    assert get_package_prefix_basename('bar') == 'prefix1', \
-        "Expected 'bar' in 'prefix2'"
-    assert get_package_prefix_basename('baz') == 'prefix2', \
-        "Expected 'baz' in 'prefix2'"
+    assert get_package_prefix_basename('foo') == 'prefix1', "Expected 'foo' in 'prefix1'"
+    assert get_package_prefix_basename('bar') == 'prefix1', "Expected 'bar' in 'prefix2'"
+    assert get_package_prefix_basename('baz') == 'prefix2', "Expected 'baz' in 'prefix2'"
 
-    try:
+    with pytest.raises(PackageNotFoundError) as excinfo:
         get_package_prefix('does_not_exist')
-    except PackageNotFoundError:
-        pass
-    except Exception as exc:
-        assert False, 'Expected PackageNotFoundError, got: {}' \
-            .format(type(exc))
-
-    try:
-        get_package_prefix('does_not_exist')
-    except KeyError:
-        pass
-    except Exception as exc:
-        assert False, 'Expected KeyError or subclass, got: {}' \
-            .format(type(exc))
+    assert excinfo.type is PackageNotFoundError, (
+        f'Expected PackageNotFoundError, got: {excinfo.type}')
 
 
 def test_get_package_share_directory():
@@ -179,126 +156,104 @@ def test_get_package_share_directory():
     def get_package_share_directory_test(package_name, expect_prefix):
         full_share_dir = get_package_share_directory(package_name)
         left_over, dirname = os.path.split(full_share_dir)
-        assert dirname == package_name, "Expected package name '{}'" \
-            .format(package_name)
+        assert dirname == package_name, f"Expected package name '{package_name}'"
         left_over, dirname = os.path.split(left_over)
         assert dirname == 'share', "Expected 'share'"
         left_over, dirname = os.path.split(left_over)
-        assert dirname == expect_prefix, "Expected '{}'".format(expect_prefix)
+        assert dirname == expect_prefix, f"Expected '{expect_prefix}'"
 
     get_package_share_directory_test('foo', 'prefix1')
-    # found in both prefix1 and prefix2, but prefix1 is ahead on the APP
     get_package_share_directory_test('bar', 'prefix1')
     get_package_share_directory_test('baz', 'prefix2')
 
-    try:
+    with pytest.raises(PackageNotFoundError) as excinfo:
         get_package_share_directory('does_not_exist')
-    except PackageNotFoundError:
-        pass
-    except Exception as exc:
-        assert False, 'Expected PackageNotFoundError, got: {}' \
-            .format(type(exc))
+    assert excinfo.type is PackageNotFoundError, (
+        f'Expected PackageNotFoundError, got: {excinfo.type}')
 
 
 def test_get_resource_types():
-    # Empty ament prefix will raise env error
-    set_ament_prefix_path([''])
-    try:
+    set_ament_prefix_path([])
+    with pytest.raises(EnvironmentError) as excinfo:
         get_resource_types()
-    except EnvironmentError:
-        pass
-    except Exception as exc:
-        assert False, 'Expected EnvironmentError, got: {}'.format(type(exc))
+    assert excinfo.type is EnvironmentError, f'Expected EnvironmentError, got: {excinfo.type}'
 
     set_ament_prefix_path(['prefix1', 'prefix2'])
     resources = get_resource_types()
-    assert resources == {'resource_type1',
-                         'resource_type2',
-                         'resource_type3',
-                         'resource_type4',
-                         'resource_type5',
-                         'packages'}, 'Expected resources to ' + \
-        'be found: resource_type1,  resource_type2, ' + \
-        'resource_type3,  resource_type4, ' + \
-        'resource_type5 and packages'
+    assert resources == {
+        'resource_type1',
+        'resource_type2',
+        'resource_type3',
+        'resource_type4',
+        'resource_type5',
+        'packages'
+    }, ('Expected resources to be: resource_type1, resource_type2, resource_type3, '
+        'resource_type4, resource_type5 and packages')
 
     set_ament_prefix_path(['prefix1'])
     resources = get_resource_types()
-    assert resources == {'resource_type1',
-                         'resource_type2',
-                         'resource_type4',
-                         'resource_type5',
-                         'packages'}, 'Expected resources to ' + \
-        'be found: resource_type1,  resource_type2, ' + \
-        'resource_type4, resource_type5 and packages'
+    assert resources == {
+        'resource_type1',
+        'resource_type2',
+        'resource_type4',
+        'resource_type5',
+        'packages'
+    }, ('Expected resources to be: resource_type1, resource_type2, resource_type4, '
+        'resource_type5 and packages')
 
 
 def test_main_tool(capsys):
     set_ament_prefix_path(['prefix1', 'prefix2'])
-    base_path = os.path.dirname(__file__)
+    base_path = Path(__file__).parents[0]
 
-    # Get output from the tool with name empty
     main()
     captured = capsys.readouterr()
-    expected_result = 'packages\n' + \
-                      'resource_type1\n' + \
-                      'resource_type2\n' + \
-                      'resource_type3\n' + \
-                      'resource_type4\n' + \
-                      'resource_type5\n'
+    expected_result = (
+        'packages\nresource_type1\nresource_type2\n'
+        'resource_type3\nresource_type4\nresource_type5\n')
     assert captured.out == expected_result
 
-    # Get output from the tool with name empty
     main(argv=['packages'])
     captured = capsys.readouterr()
-    expected_result = 'bar\t' + base_path + os.sep + 'prefix1\n' + \
-                      'baz\t' + base_path + os.sep + 'prefix2\n' + \
-                      'foo\t' + base_path + os.sep + 'prefix1\n'
+    expected_result = '\n'.join([
+        f"bar\t{base_path / 'prefix1'}",
+        f"baz\t{base_path / 'prefix2'}",
+        f"foo\t{base_path / 'prefix1'}\n",
+    ])
     assert captured.out == expected_result
 
-    # Get output using both of the variables
     main(argv=['packages', 'bar'])
     captured = capsys.readouterr()
-    expected_result = base_path + os.sep + 'prefix1\n'
+    expected_result = str(base_path / 'prefix1\n')
     assert captured.out == expected_result
 
-    # Get output using both of the variables, content
     main(argv=['resource_type4', 'foo'])
     captured = capsys.readouterr()
-    expected_result = base_path + os.sep + 'prefix1\n'
-    expected_result = expected_result + '<<<\nfoo\n>>>\n'
+    expected_result = f"{base_path / 'prefix1'}\n<<<\nfoo\n>>>\n"
     assert captured.out == expected_result
 
-    # Try getting something not available
-    resource_type = 'packages'
-    resource_name = 'not_available'
-    result = main(argv=[resource_type, resource_name])
+    result = main(argv=['packages', 'not_available'])
     captured = capsys.readouterr()
-    expected_result = "Could not find the resource '%s' of type '%s'" \
-                      % (resource_name, resource_type)
+    expected_result = "Could not find the resource 'not_available' of type 'packages'"
     assert result == expected_result
 
 
 def test_autocomplete():
     set_ament_prefix_path(['prefix1', 'prefix2'])
 
-    # Test with partial resource type
     result = sorted(resource_type_completer('res'))
     expected_result = ['resource_type1', 'resource_type2',
                        'resource_type3', 'resource_type4',
                        'resource_type5']
     assert result == expected_result
 
-    # Test with partial resource name
     class arguments():
         resource_type = 'packages'
-    # Second argument typed (ba) is not used here
+
     result = sorted(resource_name_completer('ba', arguments))
-    expected_dict = {'foo': '', 'bar': '', 'baz': ''}
-    expected_result = sorted(expected_dict.keys())
+    expected_result = ['bar', 'baz']
     assert result == expected_result
 
-    # Test with empty
     setattr(arguments, 'resource_type', None)
     result = sorted(resource_name_completer('ba', arguments))
     expected_result = []