-
Notifications
You must be signed in to change notification settings - Fork 28
test: refactor fake email heuristic to be more optimistic and allow offline tests #1154
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
base: main
Are you sure you want to change the base?
Conversation
a8aa63a
to
57421da
Compare
|
tests/integration/cases/email_checking_pypi_malware_analyzer/check_emails.sh
Outdated
Show resolved
Hide resolved
Thanks for that. But going forward, we don't need to run ShellCheck because it is added already as a pre-commit hook 🙂 |
b6d0a94
to
e38bc00
Compare
66a66fa
to
4fff31a
Compare
…ng emails Signed-off-by: Carl Flottmann <carl.flottmann@oracle.com>
Signed-off-by: Carl Flottmann <carl.flottmann@oracle.com>
…d email check on smooth-operator Signed-off-by: Carl Flottmann <carl.flottmann@oracle.com>
…iverability Signed-off-by: Carl Flottmann <carl.flottmann@oracle.com>
Signed-off-by: Carl Flottmann <carl.flottmann@oracle.com>
Signed-off-by: Carl Flottmann <carl.flottmann@oracle.com>
Signed-off-by: Carl Flottmann <carl.flottmann@oracle.com>
Signed-off-by: Carl Flottmann <carl.flottmann@oracle.com>
302c5b7
to
3768fea
Compare
"""Test the analyzer skips if no author_email or maintainer_email is present.""" | ||
pypi_package_json_asset_mock.package_json = {"info": {"author_email": None, "maintainer_email": None}} | ||
result, info = analyzer.analyze(pypi_package_json_asset_mock) | ||
@pytest.fixture(name="fake_email_defaults_override") |
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 fixture is defined but it's not used within the tests. Therefore, the logic of overriding check_deliverability = False
didn't run, meaning that all FakeEmailAnalyzer
instances in this test module had check_deliverability
set to True. I have verified this by adding a breakpoint
@pytest.fixture(name="fake_email_defaults_override")
def set_defaults_(tmp_path: Path) -> None:
"""Disable check_deliverability in defaults.ini so we do not make network connections.
Parameters
----------
tmp_path: Path
Pytest temporary path fixture.
"""
import pdb; pdb.set_trace()
defaults_file = Path(os.path.join(tmp_path, "config.ini"))
content = """
[heuristic.pypi]
check_deliverability = False
"""
defaults_file.write_text(content, encoding="utf-8")
assert load_defaults(str(defaults_file)) is True
And ran this test module with
pytest --no-cov /home/tromai/dev/oracle/github/oracle/macaron/tests/malware_analyzer/pypi/test_fake_email.py
where the breakpoint wasn't triggered.
One fix could be using this fixture in the analyzer_
fixture:
@pytest.fixture(name="analyzer")
def analyzer_(fake_email_defaults_override: None) -> FakeEmailAnalyzer:
"""Pytest fixture to create a FakeEmailAnalyzer instance."""
return FakeEmailAnalyzer()
Summary
This PR refactors the
FakeEmailAnalyzer
focusing on making it more optimistic in passing emails, providing more information, and allowing tests to be run offline.Description of changes
Previously, the
FakeEmailAnalyzer.analyze
method would return aFAIL
result on the first detected invalid, or non-email, field for both the author and maintainer emails. This PR refactors this with the goal of reducing false positives to instead only return aFAIL
if all emails are invalid, so if any of the emails are determined to be valid, then this heuristic passes. Previously, theFakeEmailAnalyzer.analyze
method would also return immediately on aFAIL
case and not analyze other emails if present. This PR refactors this to instead analyze all present emails regardless, to provide more information in its JSON return value.The tests are refactored to run offline. Unit tests only test for valid and non-emails, as to run offline checking deliverability is turned off. The integration test
email_checking_pypi_malware_analyzer
has been added to check the deliverability of emails so invalid emails can be tested. It runs against the packagesmooth-operator
, which has a known invalid email address with domainexample.com
, andemail-validator
, which has a known valid and deliverable email address.Checklist
verified
label should appear next to all of your commits on GitHub.