Skip to content

Commit

Permalink
Improve code coverage (#102)
Browse files Browse the repository at this point in the history

Changes:

    Drop codecov requirement from 85% to 70%, can be revised upward when design is more settled
    Ignore startup directory when running codecov
    Remove ... from abstract methods
    Remove context manager from message template as it was unused
    Cover message listener decorator with test
    Remove unused methods
    Test topics
    Test event streams
  • Loading branch information
callumforrester authored Mar 8, 2023
1 parent 8e95744 commit 79b0477
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 49 deletions.
4 changes: 3 additions & 1 deletion codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,7 @@ coverage:
status:
project:
default:
target: 85% # the required coverage value
target: 70% # the required coverage value
threshold: 1% # the leniency in hitting the target
ignore:
- "src/blueapi/startup"
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ testpaths = "docs src tests"

[tool.coverage.run]
data_file = "/tmp/blueapi.coverage"
omit = ["src/blueapi/startup/**/*"]

[tool.coverage.paths]
# Tests are run from installed location, map back to the src directory
source = ["src", "**/site-packages/"]
omit = ["src/blueapi/startup"]

# tox must currently be configured via an embedded ini string
# See: https://github.com/tox-dev/tox/issues/999
Expand Down
6 changes: 0 additions & 6 deletions src/blueapi/core/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ def subscribe(self, __callback: Callable[[E], None]) -> S:
S: A unique token representing the subscription
"""

...

@abstractmethod
def unsubscribe(self, __subscription: S) -> None:
"""
Expand All @@ -37,16 +35,12 @@ def unsubscribe(self, __subscription: S) -> None:
__subscription (S): The token identifying the subscription
"""

...

@abstractmethod
def unsubscribe_all(self) -> None:
"""
Unsubscribe from all subscriptions
"""

...


class EventPublisher(EventStream[E, int]):
"""
Expand Down
29 changes: 0 additions & 29 deletions src/blueapi/messaging/base.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from abc import ABC, abstractmethod
from concurrent.futures import Future
from types import TracebackType
from typing import Any, Callable, Optional, Type

from .context import MessageContext
Expand Down Expand Up @@ -28,8 +27,6 @@ def default(self, name: str) -> str:
str: Identifier for the destination
"""

...

@abstractmethod
def queue(self, name: str) -> str:
"""
Expand All @@ -42,8 +39,6 @@ def queue(self, name: str) -> str:
str: Identifier for the queue
"""

...

@abstractmethod
def topic(self, name: str) -> str:
"""
Expand All @@ -56,8 +51,6 @@ def topic(self, name: str) -> str:
str: Identifier for the topic
"""

...

@abstractmethod
def temporary_queue(self, name: str) -> str:
"""
Expand All @@ -70,8 +63,6 @@ def temporary_queue(self, name: str) -> str:
str: Identifier for the queue
"""

...


class MessagingTemplate(ABC):
"""
Expand All @@ -91,8 +82,6 @@ def destinations(self) -> DestinationProvider:
DestinationProvider: Destination provider
"""

...

def send_and_recieve(
self,
destination: str,
Expand Down Expand Up @@ -138,8 +127,6 @@ def send(
a reply. Defaults to None.
"""

...

def listener(self, destination: str):
"""
Decorator for subscribing to a topic:
Expand Down Expand Up @@ -179,30 +166,14 @@ def callback(context: MessageContext, message: ???) -> None:
__callback (MessageListener): What to do with each message
"""

...

def __enter__(self) -> "MessagingTemplate":
self.connect()
return self

def __exit__(
self,
exc_type: Optional[Type[BaseException]],
exc_val: Optional[BaseException],
exc_tb: Optional[TracebackType],
) -> None:
self.disconnect()

@abstractmethod
def connect(self) -> None:
"""
Connect the app to transport
"""
...

@abstractmethod
def disconnect(self) -> None:
"""
Disconnect the app from transport
"""
...
3 changes: 0 additions & 3 deletions src/blueapi/messaging/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,3 @@ class MessageContext:

destination: str
reply_destination: Optional[str]

def can_reply(self) -> bool:
return self.reply_destination is not None
1 change: 0 additions & 1 deletion src/blueapi/worker/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ def do_task(self, ctx: BlueskyContext) -> None:
Args:
ctx (TaskContext): Context for the task
"""
...


LOGGER = logging.getLogger(__name__)
Expand Down
8 changes: 0 additions & 8 deletions src/blueapi/worker/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,12 @@ def submit_task(self, __name: str, __task: T) -> None:
__name (str): A unique name to identify this task
__task (T): The task to run
"""
...

@abstractmethod
def run_forever(self) -> None:
"""
Run all tasks as-submitted. Blocks thread.
"""
...

@property
@abstractmethod
Expand All @@ -42,8 +40,6 @@ def worker_events(self) -> EventStream[WorkerEvent, int]:
EventStream[WorkerEvent, int]: Subscribable stream of events
"""

...

@property
@abstractmethod
def progress_events(self) -> EventStream[ProgressEvent, int]:
Expand All @@ -54,8 +50,6 @@ def progress_events(self) -> EventStream[ProgressEvent, int]:
EventStream[ProgressEvent, int]: Subscribable stream of events
"""

...

@property
@abstractmethod
def data_events(self) -> EventStream[DataEvent, int]:
Expand All @@ -65,5 +59,3 @@ def data_events(self) -> EventStream[DataEvent, int]:
Returns:
EventStream[DataEvent, int]: Subscribable stream of events
"""

...
72 changes: 72 additions & 0 deletions tests/core/test_event.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
from concurrent.futures import Future
from dataclasses import dataclass
from queue import Queue
from typing import Iterable

import pytest

from blueapi.core import EventPublisher

_TIMEOUT: float = 10.0


@dataclass
class MyEvent:
a: str


@pytest.fixture
def publisher() -> EventPublisher[MyEvent]:
return EventPublisher()


def test_publishes_event(publisher: EventPublisher[MyEvent]) -> None:
event = MyEvent("a")
f: Future = Future()
publisher.subscribe(f.set_result)
publisher.publish(event)
assert f.result(timeout=_TIMEOUT) == event


def test_multi_subscriber(publisher: EventPublisher[MyEvent]) -> None:
event = MyEvent("a")
f1: Future = Future()
f2: Future = Future()
publisher.subscribe(f1.set_result)
publisher.subscribe(f2.set_result)
publisher.publish(event)
assert f1.result(timeout=_TIMEOUT) == f2.result(timeout=_TIMEOUT) == event


def test_can_unsubscribe(publisher: EventPublisher[MyEvent]) -> None:
event_a = MyEvent("a")
event_b = MyEvent("b")
event_c = MyEvent("c")
q: Queue = Queue()
sub = publisher.subscribe(q.put)
publisher.publish(event_a)
publisher.unsubscribe(sub)
publisher.publish(event_b)
publisher.subscribe(q.put)
publisher.publish(event_c)
assert list(_drain(q)) == [event_a, event_c]


def test_can_unsubscribe_all(publisher: EventPublisher[MyEvent]) -> None:
event_a = MyEvent("a")
event_b = MyEvent("b")
event_c = MyEvent("c")
q: Queue = Queue()
publisher.subscribe(q.put)
publisher.subscribe(q.put)
publisher.publish(event_a)
publisher.unsubscribe_all()
publisher.publish(event_b)
publisher.subscribe(q.put)
publisher.publish(event_c)
assert list(_drain(q)) == [event_a, event_a, event_c]


def _drain(queue: Queue) -> Iterable:
while not queue.empty():
yield queue.get_nowait()
30 changes: 30 additions & 0 deletions tests/messaging/test_stomptemplate.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ def test_queue(template: MessagingTemplate) -> str:
return template.destinations.queue(f"test-{next(_COUNT)}")


@pytest.fixture
def test_topic(template: MessagingTemplate) -> str:
return template.destinations.topic(f"test-{next(_COUNT)}")


@pytest.mark.stomp
def test_send(template: MessagingTemplate, test_queue: str) -> None:
f: Future = Future()
Expand All @@ -41,6 +46,18 @@ def callback(ctx: MessageContext, message: str) -> None:
assert f.result(timeout=_TIMEOUT)


@pytest.mark.stomp
def test_send_to_topic(template: MessagingTemplate, test_topic: str) -> None:
f: Future = Future()

def callback(ctx: MessageContext, message: str) -> None:
f.set_result(message)

template.subscribe(test_topic, callback)
template.send(test_topic, "test_message")
assert f.result(timeout=_TIMEOUT)


@pytest.mark.stomp
def test_send_on_reply(template: MessagingTemplate, test_queue: str) -> None:
acknowledge(template, test_queue)
Expand All @@ -61,6 +78,19 @@ def test_send_and_recieve(template: MessagingTemplate, test_queue: str) -> None:
assert reply == "ack"


@pytest.mark.stomp
def test_listener(template: MessagingTemplate, test_queue: str) -> None:
@template.listener(test_queue)
def server(ctx: MessageContext, message: str) -> None:
reply_queue = ctx.reply_destination
if reply_queue is None:
raise RuntimeError("reply queue is None")
template.send(reply_queue, "ack")

reply = template.send_and_recieve(test_queue, "test", str).result(timeout=_TIMEOUT)
assert reply == "ack"


@dataclass
class Foo:
a: int
Expand Down

0 comments on commit 79b0477

Please sign in to comment.