Fix track number 0 not displaying in chat announcements #1205
Conversation
Reviewer's GuideThis PR replaces truthiness-based filtering with explicit Class diagram for updated _finalize methods and metadata parsingclassDiagram
class MetadataHandler {
+_got_tag(tag)
metadata
}
class TwitchChat {
+_finalize(variable)
}
class KickChat {
+_finalize(variable)
}
class Utils {
+_finalize(variable)
}
MetadataHandler <|-- TwitchChat
MetadataHandler <|-- KickChat
MetadataHandler <|-- Utils
%% Highlight the change: _finalize and _got_tag now use `is not None`
note for MetadataHandler "_got_tag: now uses `getattr(tag, key) is not None` to allow 0 values"
note for TwitchChat "_finalize: now uses `if variable is not None` to allow 0 values"
note for KickChat "_finalize: now uses `if variable is not None` to allow 0 values"
note for Utils "_finalize: now uses `if variable is not None` to allow 0 values"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @aw-was-here - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if track_value is not None: | ||
| metadata['track'] = track_value |
There was a problem hiding this comment.
issue (code-quality): Avoid conditionals in tests. (no-conditionals-in-tests)
Explanation
Avoid complex code, like conditionals, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
| if disc_value is not None: | ||
| metadata['disc'] = disc_value |
There was a problem hiding this comment.
issue (code-quality): Avoid conditionals in tests. (no-conditionals-in-tests)
Explanation
Avoid complex code, like conditionals, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
| if expected_track: | ||
| assert f'Track: {track_value}' in content | ||
| else: | ||
| assert 'Track:' not in content |
There was a problem hiding this comment.
issue (code-quality): Avoid conditionals in tests. (no-conditionals-in-tests)
Explanation
Avoid complex code, like conditionals, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
| if expected_disc: | ||
| assert f'Disc: {disc_value}' in content | ||
| else: | ||
| assert 'Disc:' not in content |
There was a problem hiding this comment.
issue (code-quality): Avoid conditionals in tests. (no-conditionals-in-tests)
Explanation
Avoid complex code, like conditionals, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
Root cause: metadata.py:577 used truthiness check `getattr(tag, key)` which excluded track=0 from processing. Template finalizers also converted 0 to empty string. Changes: - metadata.py: Change `getattr(tag, key)` to `getattr(tag, key) is not None` - chat.py (Twitch/Kick): Change `if variable:` to `if variable is not None:` - utils.py: Change `if variable:` to `if variable is not None:` Testing: - Add parameterized test for metadata processing of 0 values - Add parameterized test for template rendering of track=0/disc=0 - All tests pass, pylint clean Fixes issue where track 0 (common for pre-gap tracks, singles) was not announced in Twitch/Kick chat despite being valid metadata.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1205 +/- ##
==========================================
- Coverage 66.56% 66.48% -0.08%
==========================================
Files 63 63
Lines 10697 10697
==========================================
- Hits 7120 7112 -8
- Misses 3577 3585 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Root cause: metadata.py:577 used truthiness check
getattr(tag, key)which excluded track=0 from processing. Template finalizers also converted 0 to empty string.Changes:
getattr(tag, key)togetattr(tag, key) is not Noneif variable:toif variable is not None:if variable:toif variable is not None:Testing:
Fixes issue where track 0 (common for pre-gap tracks, singles) was not announced in Twitch/Kick chat despite being valid metadata.
Summary by Sourcery
Fix track number zero not displaying in chat announcements by updating conditional checks to treat 0 as a valid value and adding tests to cover zero and None cases.
Bug Fixes:
Tests: