Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

History in context #26

Merged
merged 7 commits into from
Jan 16, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion routemaster/context.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Context definition for exit condition programs."""
import datetime
from typing import Any, Dict, Iterable, Sequence
from typing import Any, Dict, Iterable, Optional, Sequence

import dateutil.tz

Expand All @@ -19,6 +19,7 @@ def __init__(
now: datetime.datetime = None,
feeds: Dict[str, Feed] = {},
accessed_variables: Iterable[str] = [],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that having these mutable defaults is a good idea (maybe add flake8-mutable?).

Perhaps a better way to solve this would be a helper used in tests which handles the non-interesting defaults?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed these with a test factory method that has the defaults (but not as mutable args).

current_history_entry: Optional[Any] = None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really not have a type for the entry? Or is it that SqlAlchemy doesn't provide a useful type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latter, because we aren't using the ORM. This will change eventually I hope.

) -> None:
"""Create an execution context."""
if now is None:
Expand All @@ -31,6 +32,7 @@ def __init__(
self.now = now
self.metadata = metadata
self.feeds = feeds
self.current_history_entry = current_history_entry

self._pre_warm_feeds(label, accessed_variables)

Expand All @@ -42,6 +44,7 @@ def lookup(self, path: Sequence[str]) -> Any:
return {
'metadata': self._lookup_metadata,
'feeds': self._lookup_feed_data,
'history': self._lookup_history,
}[location](rest)
except (KeyError, ValueError):
return None
Expand All @@ -53,6 +56,16 @@ def _lookup_feed_data(self, path: Sequence[str]) -> Any:
feed_name, *rest = path
return self.feeds[feed_name].lookup(rest)

def _lookup_history(self, path: Sequence[str]) -> Any:
if self.current_history_entry is None:
raise ValueError("Accessed uninitialised variable")

variable_name, = path
return {
'entered_state': self.current_history_entry.created,
'previous_state': self.current_history_entry.old_state,
}[variable_name]

def property_handler(self, property_name, value, **kwargs):
"""Handle a property in execution."""
if property_name == ('passed',):
Expand Down
35 changes: 34 additions & 1 deletion routemaster/exit_conditions/tests/test_integration.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import datetime
import textwrap
from typing import Optional, NamedTuple

import pytest
import dateutil.tz
Expand All @@ -19,21 +20,51 @@
("not true", False, ()),
("3h has passed since metadata.old_time", True, ('metadata.old_time',)),
("not 4 >= 6", True, ()),
("3h has not passed since metadata.old_time", False, ('metadata.old_time',)),
(
"3h has not passed since metadata.old_time",
False,
('metadata.old_time',),
),
("metadata.foo is defined", True, ('metadata.foo',)),
("metadata.bar is defined", False, ('metadata.bar',)),
("null is not defined", True, ()),
("(1 < 2) and (2 < metadata.foo)", True, ('metadata.foo',)),
("3 is not in metadata.objects", True, ('metadata.objects',)),
(
"12h has passed since history.entered_state",
True,
('history.entered_state',),
),
(
"1d12h has passed since history.entered_state",
False,
('history.entered_state',),
),
(
"history.previous_state = incorrect_state",
False,
('history.previous_state', 'incorrect_state'),
),
]


class FakeHistoryEntry(NamedTuple):
created: datetime.datetime
old_state: Optional[str]
new_state: Optional[str]


NOW = datetime.datetime(2017, 1, 1, 12, 0, 0, tzinfo=dateutil.tz.tzutc())
VARIABLES = {
'foo': 4,
'objects': (2, 4),
'old_time': NOW - datetime.timedelta(hours=4),
}
HISTORY_ENTRY = FakeHistoryEntry(
old_state='old_state',
new_state='new_state',
created=NOW - datetime.timedelta(hours=15),
)


@pytest.mark.parametrize("program, expected, variables", PROGRAMS)
Expand All @@ -43,6 +74,7 @@ def test_evaluate(program, expected, variables):
label='label1',
metadata=VARIABLES,
now=NOW,
current_history_entry=HISTORY_ENTRY,
accessed_variables=program.accessed_variables(),
)
assert program.run(context) == expected
Expand Down Expand Up @@ -144,6 +176,7 @@ def test_errors(source, error):
label='label1',
metadata=VARIABLES,
now=NOW,
current_history_entry=HISTORY_ENTRY,
accessed_variables=program.accessed_variables(),
)
program.run(context)
Expand Down
8 changes: 8 additions & 0 deletions routemaster/tests/test_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,14 @@ def test_non_existent_feed_is_none():
assert context.lookup(['feeds', 'foo', 'bar']) is None


def test_non_existent_history_variable_is_none():
context = Context(
label='label1',
accessed_variables=['history.foo'],
)
assert context.lookup(['history', 'foo']) is None


def test_accessing_prefix_directly_does_not_error():
context = Context(
label='label1',
Expand Down