Differential D5017 Diff 17906 swh/storage/tests/test_buffer.py

Changeset View

Standalone View

swh/storage/tests/test_buffer.py

# Copyright (C) 2019-2021 The Software Heritage developers		# Copyright (C) 2019-2021 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution		# See the AUTHORS file at the top-level directory of this distribution
# License: GNU General Public License version 3, or any later version		# License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information		# See top-level LICENSE file for more information

		from typing import Optional
		from unittest.mock import Mock

from swh.storage import get_storage		from swh.storage import get_storage
from swh.storage.buffer import BufferingProxyStorage		from swh.storage.buffer import BufferingProxyStorage


def get_storage_with_buffer_config(**buffer_config) -> BufferingProxyStorage:		def get_storage_with_buffer_config(**buffer_config) -> BufferingProxyStorage:
steps = [		steps = [
{"cls": "buffer", **buffer_config},		{"cls": "buffer", **buffer_config},
{"cls": "memory"},		{"cls": "memory"},
▲ Show 20 Lines • Show All 489 Lines • ▼ Show 20 Lines	def test_buffer_flush_stats(sample_data) -> None:
s = storage.flush()		s = storage.flush()
assert s["content:add"] > 0		assert s["content:add"] > 0
assert s["content:add:bytes"] > 0		assert s["content:add:bytes"] > 0
assert s["skipped_content:add"] > 0		assert s["skipped_content:add"] > 0
assert s["directory:add"] > 0		assert s["directory:add"] > 0
assert s["revision:add"] > 0		assert s["revision:add"] > 0
assert s["release:add"] > 0		assert s["release:add"] > 0
assert s["snapshot:add"] > 0		assert s["snapshot:add"] > 0


		def test_buffer_operation_order(sample_data) -> None:
		storage = get_storage_with_buffer_config()

		# Wrap the inner storage in a mock to track all method calls.
		storage.storage = mocked_storage = Mock(wraps=storage.storage)

		ardumontUnsubmitted Not Done Inline Actions Surprised a bit for the first iteration on `content_add`, i would expect an AssertionError on 0 > 0... I guess the `methods_called.index("content_add")` returns something strictly positive then (no idea really). Wouldn't it be "safer" to start prev with -1 or something? ardumont: Surprised a bit for the first iteration on `content_add`, i would expect an AssertionError on 0…
		olasdAuthorUnsubmitted Done Inline Actions So, you're totally right, but, currently, `buffer.flush()` calls through to `buffer.storage.flush()` first, /then/ pushes the objects. I suspect that's an oversight from the original implementation, and I'd argue it's a bug (even if, in practice, the buffering storage is probably the only flushable storage). Let's fix that. olasd: So, you're totally right, but, currently, `buffer.flush()` calls through to `buffer.storage.
		ardumontUnsubmitted Not Done Inline Actions oh right! oversight conceded ;) ardumont: oh right! oversight conceded ;)
		# Simulate a loader: add contents, directories, revisions, releases, then
		# snapshots.
		storage.content_add(sample_data.contents)
		storage.skipped_content_add(sample_data.skipped_contents)
		storage.directory_add(sample_data.directories)
		storage.revision_add(sample_data.revisions)
		storage.release_add(sample_data.releases)
		storage.snapshot_add(sample_data.snapshots)

		# Check that nothing has been flushed yet
		assert mocked_storage.method_calls == []

		# Flush all the things
		storage.flush()

		methods_called = [c[0] for c in mocked_storage.method_calls]
		prev = -1
		for method in [
		"content_add",
		"skipped_content_add",
		"directory_add",
		"revision_add",
		"release_add",
		"snapshot_add",
		"flush",
		]:
		try:
		cur: Optional[int] = methods_called.index(method)
		except ValueError:
		cur = None

		assert cur is not None, "Method %s not called" % method
		assert cur > prev, "Method %s called out of order; all calls were: %s" % (
		method,
		methods_called,
		)
		prev = cur