Replace global is_peekable with a check on every file

[ecederstrand]

Fixes #107

To accept your contribution, please ensure that the checklist below is complete.

• Is your name/identity in the AUTHORS file?
• Does the code change (if the PR contains code) have 100% test coverage?
• Is CI passing all quality and testing checks?

[codecov-io]

Codecov Report

Merging #108 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #108   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          12     12           
  Lines         576    573    -3     
=====================================
- Hits          576    573    -3

Impacted Files	Coverage Δ
tap/parser.py	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ab5bd7...9871339. Read the comment docs.

[mblayman]

Overall, the idea behind the fix looks good, but I think it's possible that this is trading one AttributeError for another one. I think this might be subtly wrong and indicative that the test suite doesn't have adequate coverage.

Since peekable is from more_itertools, this is going to fail with an AttributeError when more_itertools is not installed. By default, tappy does not install more_itertools without using tap.py[yaml] as the package to install.

Maybe an ENABLE_VERSION_13 if clause is needed somewhere in here.

[ecederstrand]

Good point about peekable only being available when ENABLE_VERSION_13=True. I added a commit to address this.

I don't have any idea how to improve test coverage of this, if it's required for acceptance of this PR. Do you have some pointers to get me in the right direction?

[mblayman]

Great question! I think the way we can check on this is with tox. I made this local modification to generate a coverage report.

diff --git a/tox.ini b/tox.ini
index 72d5060..79a37cf 100644
--- a/tox.ini
+++ b/tox.ini
@@ -11,8 +11,12 @@ envlist =
 
 [testenv]
 deps =
+    coverage
     pytest
-commands = pytest {envsitepackagesdir}/tap
+commands =
+    # pytest {envsitepackagesdir}/tap
+    coverage run tap/tests/run.py
+    coverage report -m --include "*/tap/*" --omit "*/tests/*"
+    coverage html
 
 [testenv:windows]
 basepython = python3.6

Once that is complete, you can open htmlcov/index.html and look at the parser.py file. I'd suggest looking at it without your change to see if the line in question is covered or uncovered. I suspect it will be uncovered.

If that's the case, then a new test could be added to reach that line of code. I suspect that it will hit the AttributeError that we discussed.

[ecederstrand]

Thanks for the hints, and sorry for the delay. I have now committed a test case that throws an AttributeError on the master branch and succeeds with this PR.

[mblayman]

Thanks for taking the time to write this extra test. I'm glad you were able to prove that master would throw an AttributeError. This looks good to me. 👍