🧪💅 Eliminate blanket # noqa comments
#36
Conversation
tests/credential_plugins_test.py
Outdated
| def test_dsv_import(self) -> None: | ||
| from awx_plugins.credentials.dsv import SecretsVault # noqa: F401 | ||
|
|
||
| # assert this module as opposed to older thycotic.secrets.vault | ||
| assert SecretsVault.__module__ == 'delinea.secrets.vault' | ||
|
|
||
| def test_tss_import(self): | ||
| from awx_plugins.credentials.tss import DomainPasswordGrantAuthorizer, PasswordGrantAuthorizer, SecretServer, ServerSecret # noqa | ||
| from awx_plugins.credentials.tss import ( # noqa: F401 | ||
| DomainPasswordGrantAuthorizer, | ||
| PasswordGrantAuthorizer, | ||
| SecretServer, | ||
| ServerSecret, | ||
| ) |
There was a problem hiding this comment.
@chrismeyersfsu do these tests actually make sense in this form with in-function imports? Other smoke tests already import them, so that would influence the in-process module cache anyway.
| def test_dsv_import(self): | ||
| from awx_plugins.credentials.dsv import SecretsVault # noqa | ||
| def test_dsv_import(self) -> None: | ||
| from awx_plugins.credentials.dsv import SecretsVault # noqa: F401 |
There was a problem hiding this comment.
@djyasin so the best action here is to try to actually fix the underlying problem. I wonder if it's okay for us to do a pytest.importorskip() (if this test is even needed / cc @chrismeyersfsu).
If that's not acceptable for some reason, let's keep it as it is in your current patch.
There was a problem hiding this comment.
@djyasin so since https://github.com/ansible/awx-plugins/actions/runs/10854294898/job/30125329367?pr=36 is green, this job needs to be removed and the check re-integrated into the unified pre-commit one.
More specifically, this means that the following bits should be removed: https://github.com/ansible/awx-plugins/blob/96fdebc/.github/workflows/ci-cd.yml#L616-L622 / https://github.com/ansible/awx-plugins/blob/96fdebc/.github/workflows/ci-cd.yml#L608C34-L608C60.
webknjaz
force-pushed
the
python-check-blanket-noqa
branch
from
113a426
to
f9eb181
Compare
webknjaz
changed the title
# noqa comments
webknjaz
force-pushed
the
python-check-blanket-noqa
branch
from
f9eb181
to
113d14c
Compare
This PR is to address the changes needed for
python-check-blanket-noqa.