Fix coverage of meta.py #910
Conversation
| @@ -248,7 +248,9 @@ def test_strategy(self): | |||
| self.responses_test([D], [C, C, C, C], [C, C, C, C]) | |||
|
|
|||
| # After long histories tit-for-tat should come into play. | |||
| self.responses_test([D], [C] * 101, [C] * 100 + [D]) | |||
| letters = 'ABEFGHIJKLMNOPQRSTUVWXYZ' | |||
There was a problem hiding this comment.
Could you add a (short) comment explaining the need for the junk history. Just want it as a pointer for when we eventually get to refactoring this test file.
meatballs
changed the title
|
I'm not sure how I feel about an impossible history here. I think I'd rather just skip the line in the tests. |
|
I'm not against that.
…On Wed, 15 Mar 2017, 15:21 Marc Harper, ***@***.***> wrote:
I'm not sure how I feel about an impossible history here. I'd rather just
skip the line in the tests.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#910 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACCGWu6FjV7tBIx8wXyLBPeVDjsnUjrkks5rmAHwgaJpZM4Md3b7>
.
|
That's essentially our choice, I believe! I can't find a genuine history that doesn't trigger the hunters before it ever gets to that line. |
|
I vote for skipping it. |
|
Given that the test is relatively simple to write, it seems a shame to skip the line. I understand the longer term concern about junk history, but that's not a problem right now and, if we introduce proper history checking, we can always define one, specific, known junk history for testing. |
|
It could be left as you have it and then when we get to #884 for this file we refactor the tests using a bespoke approach. |
|
The work you're doing at the moment is about coverage. We have loads of history mismatches which is the point of #884. I think I'm now voting for keeping it as you have it, we'll fix the history (somehow) when we get to it. Once this is in I'm happy to put it next on my list... |
|
Yep, solving one problem at a time is often the way to go! |
|
It's not an honest test though -- it's only tripping the behavior because the actions aren't real. If we had a test where the opponent action wasn't 'C' or 'D' then it would fail. I'd prefer that we modify the strategy by removing one of the hunters. |
|
I'd be ok with that, although i wonder if removing the line we're trying to test is another alternative? (I'm 99% sure it can't happen) If removing it triggers a bug then we would have a way to test it. |
axelrod/strategies/hunter.py
Outdated
| @@ -49,7 +49,7 @@ def strategy(self, opponent: Player) -> Action: | |||
|
|
|||
| def is_alternator(history: List[Action]) -> bool: | |||
| for i in range(len(history) - 1): | |||
| if history[i] == history[i + 1]: | |||
| if history[i] == history[i + 1] or history[i] not in (C, D): | |||
There was a problem hiding this comment.
I believe we can get rid of this modification now?
| @@ -258,6 +255,23 @@ def test_strategy(self): | |||
| self.responses_test([D], [C] * 101, [C] * 101) | |||
| self.responses_test([D], [C] * 101, [C] * 100 + [D]) | |||
|
|
|||
| # To test the TFT action of the strategy after 100 turns, we need to | |||
| # remove two of the hunters from its team. | |||
| # It is almost impossible to identify a history which reach 100 turns | |||
…ory" This reverts commit abd4ffa.
|
Thanks! |
|
That should put us at 100% coverage! |
No description provided.