-
Notifications
You must be signed in to change notification settings - Fork 724
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
chore: add inline noqa suppression #5159
Conversation
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.
I think this PR was entirely automated in generation? Can you include instructions about what commands were used to generate it?
Let's also include a description of all the errors that we are ignoring, and why it is safe/reasonable to do so.
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.
Discussed offline about F821 & F841. I don't particularly love allowing these, but I'm not too concerned given that it's a specific exception and not a blanket exception.
Additionally, I would say that some funky variable ordering is one of the tricky parts of that IO function 😉
Description of changes:
s2n currently shows numerous linting warnings (F401 and F811) for code that is intentionally written this way (e.g., for test fixtures or managed processes). This PR adds inline # noqa directives to suppress these warnings
Removed
lgtm [py/unused-import]
from multiple lines since noqa handles suppression (for both ruff and CodeQL)Command to Auto-Generate Inline
noqa
ruff check /home/ubuntu/s2n-tls/tests/integrationv2/{file_name} --add-noqa
on multiple files to automatically append # noqa annotations where necessary.Errors We're Suppressing & Justification
F811 (Redefinition) for managed_process – Pytest automatically injects fixtures, but Ruff incorrectly flags the manual import as a redefinition. Since the fixture is dynamically provided and not actually redefined, this warning is a false positive.
F401 (Unused Import) for managed_process – Ruff fails to recognize managed_process as used because it is passed as a function parameter. Since pytest fixtures are injected dynamically, the import is necessary, and the warning is incorrect.
F821 (Undefined) & F841 (Unused Variable) for input_view – input_view is only instantiated when stdin is registered after detecting send_marker, but since stdin isn’t initially registered, Ruff incorrectly assumes it is undefined or unused. The variable is always assigned before use.