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

Properly handle recieve_match_attributes for Stalker, simplify for LR Players, and various style fixes #1127

Merged
merged 2 commits into from
Aug 31, 2017

Conversation

marcharper
Copy link
Member

Closes #877

@@ -62,3 +62,12 @@ def test_strategy(self):
self.versus_test(opponent=axl.Cooperator(),
expected_actions=actions,
match_attributes={"length": 4})

def test_reset(self):
Copy link
Member

@drvinceknight drvinceknight Aug 27, 2017

Choose a reason for hiding this comment

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

The parent test_reset_attributes_and_history method also plays against Alternator so I don't believe this is necessary (but it does no harm). Just mentioning it in case there's a reason I'm missing.

Copy link
Member Author

Choose a reason for hiding this comment

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

The intent is to catch a regression in the new reset / __init__ methods or the set_match_attributes / receive_match_attributes chain by looking for a specific attribute that should be reset. (There may be other such tests and/or it may be in fact useless.)

Copy link
Member

Choose a reason for hiding this comment

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

Cool. Thanks. Looks good to me. 👍

Copy link
Member

@drvinceknight drvinceknight left a comment

Choose a reason for hiding this comment

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

This looks good, just the one comment about one of the tests that I don't believe is necessary (but does no harm - feel free to keep it in).

We should add something to the documentation about using this receive_match_attributes correctly (but that can be done subsequently).

@drvinceknight
Copy link
Member

(Appveyor is falling over because of some upstream connectivity issues - setting up the conda envs -, as long as one of the builds on it is fine I'm happy.)

@meatballs meatballs merged commit 4ea41d0 into master Aug 31, 2017
@meatballs meatballs deleted the match_attributes branch August 31, 2017 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants