-
Notifications
You must be signed in to change notification settings - Fork 39
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
Remove unused test variables #3033
base: master
Are you sure you want to change the base?
Conversation
Quality Gate passedIssues Measures |
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports |
Test results 7 files 7 suites 4m 31s ⏱️ For more details on these failures, see this check. Results for commit 3650463. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3033 +/- ##
=======================================
Coverage 60.47% 60.47%
=======================================
Files 605 605
Lines 43816 43816
Branches 48 48
=======================================
Hits 26497 26497
Misses 17307 17307
Partials 12 12 ☔ View full report in Codecov by Sentry. |
@@ -84,7 +86,7 @@ def test_three_lines_with_two_rows_should_be_counted_as_three(self): | |||
b"room1:10.0.0.187:myorg:OTHER:SNMP v2c read profile::\n" | |||
) | |||
b = bulkparse.NetboxBulkParser(data) | |||
out_data = list(b) | |||
itertools.consume(b) |
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.
That failed pretty fast. I know this was my suggestion, but I must have hallucinated it. I might have built such thing myself at some point. While modules like itertools
have lots of little utilities to deal with iterators, this wasn't one of them.
You could use collections.deque
with a maximum length of 0 as a fast way to explicitly exhaust a generator, although it doesn't read as well:
itertools.consume(b) | |
deque(b, maxlen=0) |
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.
or, you could keep using the original list statement and add a short comment as to why, so people don't read it and scratch their heads...
Just like #3028 and #3029 another part in adding
F841
to ruff.