-
Notifications
You must be signed in to change notification settings - Fork 264
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
Refactor all strategy tests #884
Comments
Refactor tests for appeaser for #884
Hi I would like to help with this. Its my first contribution, any guidance how to start, I will much appreciate it. |
Hi @alajara, welcome! We have introduced a new testing framework for strategies, you can read about this here: http://axelrod.readthedocs.io/en/latest/tutorials/contributing/strategy/writing_test_for_the_new_strategy.html So this issue is about rewriting the various tests that have already been written for strategies using the new framework. Here are some PRs that I've written that do this:
So the idea is to look at the files in I've assumed there that you're ok with the notion of automated testing but please let me know if you're not and I'd be happy to walk you through anything/everything :) To help coordinate (I'm slowly working on these myself) perhaps once you're ready to go, comment on this issue when you start working on a particular test file (just so we don't work on the same one). Also, to help keep the reviews simple: let's stick to just one file per PR. Let me know if anything I've said is unclear :) Looking forward to your first contribution! |
Hello Dr Knight, I'm also interested in helping with this. should i simply write a comment on which strategy i'm doing? This is my first time contributing to someone else's project. while trying to understand the process, i came across a possible issue with backstabber.py The DoubleCrosser class does not follow its documentation and the test to demonstrate it doesn't seem to do anything. i'm not sure if this is the appropriate place for this, but here are code snippets. backstabber.py
the test strategy doesn't seem to show this. here's from test_backstabber.py
if i understand these tests correctly, DoubleCrosser will always return C vs a MockPlayer who always returns C (except for one D in round 7 on the second test). This is no different from the other class BackStabber. -Eric Shaw |
Hi @eric-s-s, Welcome to the Axelrod-Python Project! I'm Tom the original author of Backstabber and Doublecrosser. The current code for Doublecrosser is very old and was written when I was very new to Python. As far as I can tell your refactor does do exactly the same thing (and in a much more elegant way). You say that due to the tests you have shown, Backstabber and Doublecrosser do the same thing. From the situation described in the tests, they are supposed to. The difference between the two strategies should only come to light when an opponent did not defect in the first 6 turns and then subsequently defects more than 3 times (not consecutively). In this very specific situation, Doublecrosser will continue to cooperate whereas Backstabber will register the 4 defects and defect in response. The tests that you showed did not highlight this situation and hence made it look like they were doing the same thing (which in that situation they are). Hope this helps to clear things up! Tom. |
Hey Dr. Knight I can begin working on worse and worse, if it's open. |
@uglyfruitcake excellent. thank you very much. while i'm changing from self.response_test to self.versus_test i'll put that test situation in there. just to confirm for DoubleCrosser opponent = 6 * [C] + 4 * [D] -> next move (and all other moves or will it revert if opponent cooperates?) = D |
Not quite. Here's what the tests should be I think. 6 * [C] + 2 * [D] -> D Between round 7 - 180 it will only defect if the opponents previous 2 were [D, D] or if the opponent defected in the first 6 rounds. Nothing else is relevant. After 180 rounds it is EXACTLY like Backstabber. No difference. Hope that helps. Tom |
Since there are several people working on this issue please just state here which strategies you are handling. Feel free to get started wherever you like and open a PR when you think you are ready to merge. |
working on backstabber @uglyfruitcake thank you. please review the PR i'll post later. DoubleCrosser had an off-by-one error. so i ?fixed? it and did a little refactor. @marcharper sorry. i am new to all this, so i'm not sure where/how to raise the issue of test coverage/bug on a specific strategy. |
@eric-s-s you can just go ahead and address it in the PR under this issue (no need for a new issue). We'll discuss anything relevant on the PR itself :) |
Please do @alajara! :) |
i'll work on the b's and c's. |
Picking up |
Picking up |
Picking up |
Mainly using seeded tests that hit all states (added some tests that were missing as well). Addresses #884
Picking up |
This is a minor refactor of some warnings that were occurring: - A `simulate_play` that was using a fake history; - A warning from the generator equality The change to the way the generator equality was being run is based on this error: ```python .........../home/vince/src/Axelrod/axelrod/player.py:165: PendingDeprecationWarning: generator 'Player.__eq__.<locals>.<genexpr>' raised StopIteration for _ in range(200))): ``` Addresses #884
I believe #1035 might be the last of them. Will wait to adjust as required and get them all merged before taking stock 👍 |
Mainly using seeded tests that hit all states (added some tests that were missing as well). Addresses #884
This is a minor refactor of some warnings that were occurring: - A `simulate_play` that was using a fake history; - A warning from the generator equality The change to the way the generator equality was being run is based on this error: ```python .........../home/vince/src/Axelrod/axelrod/player.py:165: PendingDeprecationWarning: generator 'Player.__eq__.<locals>.<genexpr>' raised StopIteration for _ in range(200))): ``` Addresses #884
All the strategy tests have been refactored. I'm going to refactor some of the boiler plate code in |
Thanks to everyone for your efforts on this! Closed by #1036 |
With the merge of #875 we have a new method for testing strategies called
versus_test
(http://axelrod.readthedocs.io/en/latest/tutorials/contributing/strategy/writing_test_for_the_new_strategy.html). The goal is for this to eventually replace the currently usedresponses_test
.This will take a slow refactor of all strategy tests. Ideally any PRs towards this issue should only do a small number of modules at a time to ensure that nothing is missed during the review process.
EDIT: Once all of these are complete, we can refactor the
TestPlayer
class to remove unused methods.The text was updated successfully, but these errors were encountered: