Skip to content

Commit

Permalink
Use plain assertions in tests
Browse files Browse the repository at this point in the history
Pytest analyses them to give nice errors so we don't need to use
unittest methods
  • Loading branch information
giftig committed Jan 8, 2024
1 parent 2cdc44e commit f2c942c
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 164 deletions.
51 changes: 27 additions & 24 deletions s3_browser/tests/test_bookmarks.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import json
import os
import unittest
import uuid

import pytest

from s3_browser import bookmarks


class BookmarksTest(unittest.TestCase):
class TestBookmarks:
FILE_PREFIX = 's3_browser_tests_'

data = {
Expand All @@ -19,8 +20,10 @@ class BookmarksTest(unittest.TestCase):
}
}

def tearDown(self):
@pytest.fixture(scope="class", autouse=True)
def tear_down(self):
"""Clean up temporary bookmark files"""
yield
for f in os.listdir('/tmp'):
if not f.startswith(self.FILE_PREFIX):
continue
Expand Down Expand Up @@ -57,7 +60,7 @@ def test_read_bookmarks_file(self):

manager = bookmarks.BookmarkManager(f)
actual = self.normalise_bookmarks(manager.bookmarks)
self.assertEqual(actual, self.expected_bookmarks)
assert actual == self.expected_bookmarks

def test_clean_bookmark_data(self):
"""Should ignore unexpected fields in the bookmark file"""
Expand All @@ -68,13 +71,13 @@ def test_clean_bookmark_data(self):

manager = bookmarks.BookmarkManager(f)
actual = self.normalise_bookmarks(manager.bookmarks)
self.assertEqual(actual, self.expected_bookmarks)
assert actual == self.expected_bookmarks

def test_missing_bookmark_file(self):
"""Should create an empty bookmark manager if file is missing"""
f = self.gen_filename()
manager = bookmarks.BookmarkManager(f)
self.assertEqual(manager.bookmarks, {})
assert manager.bookmarks == {}

def test_add_bookmarks(self):
f = self.gen_filename()
Expand All @@ -84,34 +87,34 @@ def test_add_bookmarks(self):
manager.add_bookmark('baz', '/hodor')

actual = manager.bookmarks
self.assertEqual(actual.keys(), {'foo', 'bar', 'baz'})
self.assertEqual(
{v.path for v in actual.values()},
assert actual.keys() == {'foo', 'bar', 'baz'}
assert (
{v.path for v in actual.values()} ==
{'/hodor', '/hodor/hodor', '/hodor/hodor/hodor'}
)

for v in actual.values():
self.assertIsNotNone(v.created_on)
assert v.created_on is not None

def test_remove_bookmarks(self):
f = self.gen_filename()
self.write_fixture(f)
manager = bookmarks.BookmarkManager(f)

actual = self.normalise_bookmarks(manager.bookmarks)
self.assertEqual(actual, self.expected_bookmarks)
assert actual == self.expected_bookmarks

for b in self.data['bookmarks'].keys():
self.assertTrue(manager.remove_bookmark(b))
manager.remove_bookmark(b) is True

actual = self.normalise_bookmarks(manager.bookmarks)
self.assertEqual(actual, {})
assert actual == {}

def test_remove_missing_bookmark(self):
f = self.gen_filename()
man = bookmarks.BookmarkManager(f)
man.add_bookmark('awesome_bookmark', 'amazing/path')
self.assertFalse(man.remove_bookmark('lame_bookmark'))
assert man.remove_bookmark('lame_bookmark') is False

def test_save_bookmarks(self):
f = self.gen_filename()
Expand All @@ -129,22 +132,22 @@ def test_save_bookmarks(self):
expected = self.normalise_bookmarks(man1.bookmarks)

# Check the second instance is indeed empty
self.assertEqual(self.normalise_bookmarks(man2.bookmarks), {})
assert self.normalise_bookmarks(man2.bookmarks) == {}

# Now reload from disk and check it's the same as we just saved
man2.load()
self.assertEqual(self.normalise_bookmarks(man2.bookmarks), expected)
assert self.normalise_bookmarks(man2.bookmarks) == expected

def test_validate_bookmark_key(self):
"""Key names should be checked against a pattern"""
valid_names = ['hodor', 'ostrich', 'potato123', 'dashy-key']
invalid_names = ['thisnameisabittoolong', 'funny/characters', '-flag']

for n in valid_names:
self.assertTrue(bookmarks.BookmarkManager.validate_key(n))
assert bookmarks.BookmarkManager.validate_key(n) is True

for n in invalid_names:
self.assertFalse(bookmarks.BookmarkManager.validate_key(n))
assert bookmarks.BookmarkManager.validate_key(n) is False

def test_unreadable_bookmark_file(self):
"""If the bookmark file is unreadable, load no bookmarks"""
Expand All @@ -153,8 +156,8 @@ def test_unreadable_bookmark_file(self):
os.chmod(f, 0o200) # TODO: Cross-platform solution?

man = bookmarks.BookmarkManager(f)
self.assertIsNone(man.bookmarks)
self.assertFalse(man.add_bookmark('nope', 'nope/nope/nope'))
assert man.bookmarks is None
assert man.add_bookmark('nope', 'nope/nope/nope') is False

def test_malformed_bookmark_file(self):
"""If the JSON is malformed, refuse to support bookmarks"""
Expand All @@ -163,14 +166,14 @@ def test_malformed_bookmark_file(self):
ff.write('bad json')

man = bookmarks.BookmarkManager(f)
self.assertIsNone(man.bookmarks)
self.assertFalse(man.add_bookmark('nope', 'nope/nope/nope'))
assert man.bookmarks is None
assert man.add_bookmark('nope', 'nope/nope/nope') is False

def test_bad_bookmark_file_data(self):
"""If the JSON has a bad structure, refuse to support bookmarks"""
f = self.gen_filename()
self.write_fixture(f, {'bookmarks': 'should be an object!'})

man = bookmarks.BookmarkManager(f)
self.assertIsNone(man.bookmarks)
self.assertFalse(man.add_bookmark('nope', 'nope/nope/nope'))
assert man.bookmarks is None
assert man.add_bookmark('nope', 'nope/nope/nope') is False
47 changes: 24 additions & 23 deletions s3_browser/tests/test_completion.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import os
import shlex
import unittest
from unittest.mock import MagicMock
from unittest.mock import patch

import pytest

from s3_browser.cli import Cli
from s3_browser.completion import CliCompleter
from s3_browser.paths import S3Key
from s3_browser.paths import S3Prefix


class CompletionTestCase(unittest.TestCase):
class TestCompletion:
def _completer(self):
"""Create a CLI completer and return both it and the mocked CLI"""
m = MagicMock()
Expand All @@ -33,15 +34,15 @@ def test_complete_empty_command(self, mock):
completer = self._completer()

for i, cmd in enumerate(Cli.RECOGNISED_COMMANDS):
self.assertEqual(self._complete(completer, mock, '', i), cmd)
assert self._complete(completer, mock, '', i) == cmd

@patch('readline.get_line_buffer')
def test_complete_partial_command(self, mock):
completer = self._completer()
self.assertEqual(self._complete(completer, mock, 'c', 0), 'cat')
self.assertEqual(self._complete(completer, mock, 'c', 1), 'cd')
self.assertEqual(self._complete(completer, mock, 'c', 2), 'clear')
self.assertEqual(self._complete(completer, mock, 'bo', 0), 'bookmark')
assert self._complete(completer, mock, 'c', 0) == 'cat'
assert self._complete(completer, mock, 'c', 1) == 'cd'
assert self._complete(completer, mock, 'c', 2) == 'clear'
assert self._complete(completer, mock, 'bo', 0) == 'bookmark'

@patch('readline.get_line_buffer')
def test_complete_s3_path_commands(self, mock):
Expand All @@ -58,28 +59,28 @@ def test_complete_s3_path_commands(self, mock):
completer.cli.client.ls.return_value = prefixes + files

for i, p in enumerate(expected_paths):
self.assertEqual(self._complete(completer, mock, 'cd ', i), p)
self.assertEqual(self._complete(completer, mock, 'ls ', i), p)
self.assertEqual(self._complete(completer, mock, 'll ', i), p)
assert self._complete(completer, mock, 'cd ', i) == p
assert self._complete(completer, mock, 'ls ', i) == p
assert self._complete(completer, mock, 'll ', i) == p

for i, f in enumerate(expected_files):
self.assertEqual(self._complete(completer, mock, 'cat ', i), f)
self.assertEqual(self._complete(completer, mock, 'cat ./ ', i), f)
self.assertEqual(self._complete(completer, mock, 'file ', i), f)
self.assertEqual(self._complete(completer, mock, 'get ', i), f)
self.assertEqual(self._complete(completer, mock, 'head ', i), f)
self.assertEqual(self._complete(completer, mock, 'put ./ ', i), f)
self.assertEqual(self._complete(completer, mock, 'rm ', i), f)
self.assertEqual(self._complete(completer, mock, 'rm ./ ', i), f)
assert self._complete(completer, mock, 'cat ', i) == f
assert self._complete(completer, mock, 'cat ./ ', i) == f
assert self._complete(completer, mock, 'file ', i) == f
assert self._complete(completer, mock, 'get ', i) == f
assert self._complete(completer, mock, 'head ', i) == f
assert self._complete(completer, mock, 'put ./ ', i) == f
assert self._complete(completer, mock, 'rm ', i) == f
assert self._complete(completer, mock, 'rm ./ ', i) == f

# . and .. should suggest the relative dirs and also any s3 key hits
# Note that it'd be limited to dot-prefixed paths in reality, but our
# mock always returns expected_files for a key search in this case
for i, f in enumerate(['./'] + expected_files):
self.assertEqual(self._complete(completer, mock, 'cat .', i), f)
assert self._complete(completer, mock, 'cat .', i) == f

for i, f in enumerate(['../'] + expected_files):
self.assertEqual(self._complete(completer, mock, 'cat ..', i), f)
assert self._complete(completer, mock, 'cat ..', i) == f

@patch('readline.get_line_buffer')
def test_complete_local_path(self, mock):
Expand All @@ -88,8 +89,8 @@ def test_complete_local_path(self, mock):

files = [shlex.quote(f) for f in os.listdir('.')]
for i, f in enumerate(files):
self.assertEqual(self._complete(completer, mock, 'put ', i), f)
self.assertEqual(self._complete(completer, mock, 'get . ', i), f)
assert self._complete(completer, mock, 'put ', i) == f
assert self._complete(completer, mock, 'get . ', i) == f

@patch('readline.get_line_buffer')
def test_complete_paths_with_quotes(self, mock):
Expand All @@ -110,4 +111,4 @@ def test_complete_paths_with_quotes(self, mock):
expected = '\'argh spaces.txt\''

for p in partials:
self.assertEqual(self._complete(completer, mock, p, 0), expected)
assert self._complete(completer, mock, p, 0) == expected
115 changes: 57 additions & 58 deletions s3_browser/tests/test_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,73 +6,72 @@
from s3_browser.paths import S3Prefix


class PathsTest(unittest.TestCase):
def _test_s3_object_api(self, obj):
"""
Check that the given object satisfies the s3 object API
def _test_s3_object_api(obj):
"""
Check that the given object satisfies the s3 object API
It must have all the common properties required by the CLI to render
it in a few ways, and it must render without error both with and
without a bookmark annotation declared
"""
def basic_checks():
self.assertIsNotNone(obj.is_key)
self.assertIsNotNone(obj.full_details)
self.assertIsNotNone(obj.path_string)
It must have all the common properties required by the CLI to render
it in a few ways, and it must render without error both with and
without a bookmark annotation declared
"""
def basic_checks():
assert obj.is_key is not None
assert obj.full_details is not None
assert obj.path_string is not None

basic_checks()
obj.bookmark = 'my_bookmark'
basic_checks()
basic_checks()
obj.bookmark = 'my_bookmark'
basic_checks()

def test_s3_path_from_path_string(self):
"""S3Path should be created properly from various path strings"""
tests = [
('', S3Path(None, None)),
('/', S3Path(None, None)),
('a/b/c/d/e/f/g', S3Path('a', 'b/c/d/e/f/g')),
('/hodor-hodor', S3Path('hodor-hodor', None)),
('s3://hodor-hodor', S3Path('hodor-hodor', None)),
(
's3://hodorhodor/hodor/hodor/hodor.txt',
S3Path('hodorhodor', 'hodor/hodor/hodor.txt')
)
]
def test_s3_path_from_path_string():
"""S3Path should be created properly from various path strings"""
tests = [
('', S3Path(None, None)),
('/', S3Path(None, None)),
('a/b/c/d/e/f/g', S3Path('a', 'b/c/d/e/f/g')),
('/hodor-hodor', S3Path('hodor-hodor', None)),
('s3://hodor-hodor', S3Path('hodor-hodor', None)),
(
's3://hodorhodor/hodor/hodor/hodor.txt',
S3Path('hodorhodor', 'hodor/hodor/hodor.txt')
)
]

for input, expected in tests:
self.assertEqual(S3Path.from_path(input), expected)
for input, expected in tests:
assert S3Path.from_path(input) == expected

def test_s3_path_short_format(self):
"""S3Path should render a concise format for ease of use in prompts"""
tests = [
('/', '/'),
('a/b/c/d/e/f/g', 'a/…/g'),
(
'something-pretty-long/middle/end-of-long-thing',
'something-pretty-long/…/end-of-long-thing' # TODO: improve?
),
('foo/bar', 'foo/bar')
]
def test_s3_path_short_format():
"""S3Path should render a concise format for ease of use in prompts"""
tests = [
('/', '/'),
('a/b/c/d/e/f/g', 'a/…/g'),
(
'something-pretty-long/middle/end-of-long-thing',
'something-pretty-long/…/end-of-long-thing' # TODO: improve?
),
('foo/bar', 'foo/bar')
]

for input, expected in tests:
self.assertEqual(S3Path.from_path(input).short_format, expected)
for input, expected in tests:
assert S3Path.from_path(input).short_format == expected

def test_s3_bucket_api(self):
"""S3Bucket should support the defined S3 object API"""
bucket = S3Bucket('westeros')
def test_s3_bucket_api():
"""S3Bucket should support the defined S3 object API"""
bucket = S3Bucket('westeros')

self._test_s3_object_api(bucket)
self.assertFalse(bucket.is_key)
_test_s3_object_api(bucket)
assert bucket.is_key is False

def test_s3_prefix_api(self):
"""S3Prefix should support the defined S3 object API"""
prefix = S3Prefix('winterfell/stark')
def test_s3_prefix_api():
"""S3Prefix should support the defined S3 object API"""
prefix = S3Prefix('winterfell/stark')

self._test_s3_object_api(prefix)
self.assertFalse(prefix.is_key)
_test_s3_object_api(prefix)
assert prefix.is_key is False

def test_s3_key_api(self):
"""S3Key should support the defined S3 object API"""
key = S3Key('winterfell/stark/arya.json')
def test_s3_key_api():
"""S3Key should support the defined S3 object API"""
key = S3Key('winterfell/stark/arya.json')

self._test_s3_object_api(key)
self.assertTrue(key.is_key)
_test_s3_object_api(key)
assert key.is_key is True
Loading

0 comments on commit f2c942c

Please sign in to comment.