From 85c9c3aeee4c2afa34b90dad943ba54eddc145ad Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Mon, 20 Aug 2018 13:29:09 -0400 Subject: [PATCH 1/2] Have repository_tool.get_filepaths_in_directory use absolute paths as its docstring says that it does. I'm not sure if this changed through some accident along the way, but in any case, before this commit, the docstring said that it yielded a list of absolute paths, but it did not. Now it does. Signed-off-by: Sebastien Awwad --- tuf/repository_tool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tuf/repository_tool.py b/tuf/repository_tool.py index 68d6f65704..9f5a4158a5 100755 --- a/tuf/repository_tool.py +++ b/tuf/repository_tool.py @@ -521,7 +521,7 @@ def get_filepaths_in_directory(files_directory, recursive_walk=False, for dirpath, dirnames, filenames in os.walk(files_directory, followlinks=followlinks): for filename in filenames: - full_target_path = os.path.join(dirpath, filename) + full_target_path = os.path.join(os.path.abspath(dirpath), filename) targets.append(full_target_path) # Prune the subdirectories to walk right now if we do not wish to From b8828aebd799d47d3a07925cceddd10fc2308527 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Tue, 21 Aug 2018 12:30:25 -0400 Subject: [PATCH 2/2] Fix test that failed to detect issue with get_filepaths_in_directory The test for repository_tool.get_filepaths_in_directory now expects absolute paths, and also now tests the *results* of the function's use when the recursive flag is on. Signed-off-by: Sebastien Awwad --- tests/test_repository_tool.py | 43 +++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/tests/test_repository_tool.py b/tests/test_repository_tool.py index 4bb0f75974..b2b0827fcf 100755 --- a/tests/test_repository_tool.py +++ b/tests/test_repository_tool.py @@ -377,21 +377,50 @@ def test_get_filepaths_in_directory(self): repo = repo_tool.Repository metadata_directory = os.path.join('repository_data', 'repository', 'metadata') + # Verify the expected filenames. get_filepaths_in_directory() returns # a list of absolute paths. metadata_files = repo.get_filepaths_in_directory(metadata_directory) - expected_files = ['1.root.json', 'root.json', - 'targets.json', 'snapshot.json', - 'timestamp.json', 'role1.json', 'role2.json'] - basenames = [] - for filepath in metadata_files: - basenames.append(os.path.basename(filepath)) - self.assertEqual(sorted(expected_files), sorted(basenames)) + # Construct list of file paths expected, determining absolute paths. + expected_files = [] + for filepath in ['1.root.json', 'root.json', 'targets.json', + 'snapshot.json', 'timestamp.json', 'role1.json', 'role2.json']: + expected_files.append(os.path.abspath(os.path.join( + 'repository_data', 'repository', 'metadata', filepath))) + + self.assertEqual(sorted(expected_files), sorted(metadata_files)) + # Test when the 'recursive_walk' argument is True. + # In this case, recursive walk should yield the same results as the + # previous, non-recursive call. metadata_files = repo.get_filepaths_in_directory(metadata_directory, recursive_walk=True) + self.assertEqual(sorted(expected_files), sorted(metadata_files)) + + # And this recursive call from the directory above should yield the same + # results as well, plus extra files. + metadata_files = repo.get_filepaths_in_directory( + os.path.join('repository_data', 'repository'), recursive_walk=True) + for expected_file in expected_files: + self.assertIn(expected_file, metadata_files) + # self.assertEqual(sorted(expected_files), sorted(metadata_files)) + + # Now let's check it against the full list of expected files for the parent + # directory.... We'll add to the existing list. Expect the same files in + # metadata.staged/ as in metadata/, and a few target files in targets/ + # This is somewhat redundant with the previous test, but together they're + # probably more future-proof. + for filepath in ['file1.txt', 'file2.txt', 'file3.txt']: + expected_files.append(os.path.abspath(os.path.join( + 'repository_data', 'repository', 'targets', filepath))) + for filepath in [ '1.root.json', 'root.json', 'targets.json', + 'snapshot.json', 'timestamp.json', 'role1.json', 'role2.json']: + expected_files.append(os.path.abspath(os.path.join( + 'repository_data', 'repository', 'metadata.staged', filepath))) + + self.assertEqual(sorted(expected_files), sorted(metadata_files)) # Test improperly formatted arguments. self.assertRaises(securesystemslib.exceptions.FormatError, repo.get_filepaths_in_directory,