-
Notifications
You must be signed in to change notification settings - Fork 263
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
Add missing tests for lookerup.py #900
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me. 👍
FYI, line 157 is apparently still not covered: https://coveralls.io/builds/10562402/source?filename=axelrod%2Fstrategies%2Flookerup.py#L157 (Can wait for another PR of course.) |
I missed line 157 - I thought it had gone to 100%, but I was looking at test_lookerup.py :-( I'll sort it on this PR... |
Looks like covering line 157 is more tricky than I thought... |
Ah. I thought I could just pass a zero value for op_plays in the init_args, but there's line 122 which calcs it from the lookup table. This could take a while... |
@@ -28,6 +27,31 @@ class TestLookerUp(TestPlayer): | |||
expected_class_classifier = copy.copy(expected_classifier) | |||
expected_class_classifier['memory_depth'] = float('inf') | |||
|
|||
def test_create_lookup_table_keys(self): | |||
expected = [ | |||
('C', 'C', 'C'), ('C', 'C', 'D'), ('C', 'D', 'C'), ('C', 'D', 'D'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use C, D = Actions.C, Actions.D
throughout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with what was already there in the existing test, but yes, you're right. I'll fix the whole lot.
No description provided.