Fix _FakeServer to immediately close client connections

[cuu508]

What do these changes do?

This implements _FakeServer.connection_made() which immediately closes client connections.

Without this, if the SMTP server throws an exception during initialization, Controller.stop() gets stuck indefinitely waiting on active connections.

Note: this only happens in Python 3.12+. Earlier versions of Python allowed wait_closed() to complete regardless of active connections.

With this change the TestFactory testcases can again be enabled for Python 3.12+.

Are there changes in behavior for the user?

No change for the happy path, as _FakeServer is only used if the SMTP server throws exception during initialization.

Related issue number

Fixes: #394

Checklist

• [x] I think the code is well written
• [x] Unit tests for the changes exist
• [ ] tox testenvs have been executed in the following environments:
  • [x] Linux (Ubuntu 24.04): {py312}
  • [ ] Windows (7, 10): {py36,py37,py38,py39}-{nocov,cov,diffcov}
  • [ ] WSL 1.0 (Ubuntu 18.04): {py36,py37,py38,py39}-{nocov,cov,diffcov}, pypy3-{nocov,cov}, qa, docs
  • [ ] FreeBSD (12.2, 12.1, 11.4): {py36,pypy3}-{nocov,cov,diffcov}, qa
  • [ ] Cygwin: py36-{nocov,cov,diffcov}, qa, docs
• [ ] Documentation reflects the changes
• [ ] Add a news fragment into the NEWS.rst file

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 97.77%. Comparing base (f40ac96) to head (96f7a17). :warning: Report is 44 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #524      +/-   ##
==========================================
- Coverage   97.86%   97.77%   -0.10%     
==========================================
  Files          23       23              
  Lines        5707     5696      -11     
  Branches      764      763       -1     
==========================================
- Hits         5585     5569      -16     
- Misses         76       82       +6     
+ Partials       46       45       -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cuu508]

To maintainers, I'm happy to provide more information on what this does, or make any changes needed.

[waynew]

Looks like we had a codecov issue; probably due to no longer skipping the one test :thinking: Or perhaps the added code isn't being hit. I'm guessing this is actually the right way to fix this but it's been a while since I've been in the codebase :slightly_smiling_face:

[cuu508]

I ran tests like so on my local system:

pytest -p no:warnings

When running it on the master branch, it prints the following coverage report:

---------- coverage: platform linux, python 3.12.3-final-0 -----------
Name                                   Stmts   Miss Branch BrPart  Cover
------------------------------------------------------------------------
aiosmtpd/__init__.py                      15      0      4      1    95%
aiosmtpd/__main__.py                       3      3      2      0     0%
aiosmtpd/controller.py                   221     10     70      3    96%
aiosmtpd/handlers.py                     176      0     66      0   100%
aiosmtpd/lmtp.py                          12      0      4      0   100%
aiosmtpd/main.py                         123      2     38      0    99%
aiosmtpd/proxy_protocol.py               325      0    131      0   100%
aiosmtpd/qa/__init__.py                    0      0      0      0   100%
aiosmtpd/qa/test_0packaging.py            67     11     26      7    81%
aiosmtpd/qa/test_1testsuite.py            46      2     18      2    94%
aiosmtpd/smtp.py                         923      0    380      0   100%
aiosmtpd/testing/__init__.py               0      0      0      0   100%
aiosmtpd/testing/helpers.py               36      0      2      0   100%
aiosmtpd/testing/statuscodes.py          103      0      0      0   100%
aiosmtpd/tests/__init__.py                 0      0      0      0   100%
aiosmtpd/tests/certs/__init__.py           0      0      0      0   100%
aiosmtpd/tests/conftest.py               160     32     56      3    79%
aiosmtpd/tests/test_handlers.py          493     10    100      6    97%
aiosmtpd/tests/test_lmtp.py               33      0      2      0   100%
aiosmtpd/tests/test_main.py              246     40     70      2    83%
aiosmtpd/tests/test_misc.py               35      3      6      1    90%
aiosmtpd/tests/test_proxyprotocol.py     666      9    142      8    97%
aiosmtpd/tests/test_server.py            409     48    101      5    88%
aiosmtpd/tests/test_smtp.py             1249     11    237      3    99%
aiosmtpd/tests/test_smtps.py              34      0      6      0   100%
aiosmtpd/tests/test_smtpsmuggling.py      60      9     18      8    78%
aiosmtpd/tests/test_starttls.py          262      0     32      0   100%
------------------------------------------------------------------------
TOTAL                                   5697    190   1511     49    96%

On the PR branch it prints:

---------- coverage: platform linux, python 3.12.3-final-0 -----------
Name                                   Stmts   Miss Branch BrPart  Cover
------------------------------------------------------------------------
aiosmtpd/__init__.py                      15      0      4      1    95%
aiosmtpd/__main__.py                       3      3      2      0     0%
aiosmtpd/controller.py                   223      3     70      0    99%
aiosmtpd/handlers.py                     176      0     66      0   100%
aiosmtpd/lmtp.py                          12      0      4      0   100%
aiosmtpd/main.py                         123      2     38      0    99%
aiosmtpd/proxy_protocol.py               325      0    131      0   100%
aiosmtpd/qa/__init__.py                    0      0      0      0   100%
aiosmtpd/qa/test_0packaging.py            67     11     26      7    81%
aiosmtpd/qa/test_1testsuite.py            46      2     18      2    94%
aiosmtpd/smtp.py                         923      0    380      0   100%
aiosmtpd/testing/__init__.py               0      0      0      0   100%
aiosmtpd/testing/helpers.py               36      0      2      0   100%
aiosmtpd/testing/statuscodes.py          103      0      0      0   100%
aiosmtpd/tests/__init__.py                 0      0      0      0   100%
aiosmtpd/tests/certs/__init__.py           0      0      0      0   100%
aiosmtpd/tests/conftest.py               160     21     56      5    86%
aiosmtpd/tests/test_handlers.py          493     10    100      6    97%
aiosmtpd/tests/test_lmtp.py               33      0      2      0   100%
aiosmtpd/tests/test_main.py              246     40     70      2    83%
aiosmtpd/tests/test_misc.py               35      3      6      1    90%
aiosmtpd/tests/test_proxyprotocol.py     666      9    142      8    97%
aiosmtpd/tests/test_server.py            408      3     98      5    98%
aiosmtpd/tests/test_smtp.py             1249     11    237      3    99%
aiosmtpd/tests/test_smtps.py              34      0      6      0   100%
aiosmtpd/tests/test_smtpsmuggling.py      60      9     18      8    78%
aiosmtpd/tests/test_starttls.py          262      0     32      0   100%
------------------------------------------------------------------------
TOTAL                                   5698    127   1508     48    97%

The coverage increased--which makes sense as the PR re-enables some previously disabled tests.

I'm not familiar with aiosmtpd build and CI infrastructure, and am not sure why codecov reports different results. Perhaps it's using code coverage from a test run with a different python interpreter version? (my local system has Python 3.12)

[cuu508]

I thought I found unused code (_FakeServer._cb_client_connected) and removed it to increase coverage. But really I have no idea where the coverage discrepancy comes from, and will hold off from making more changes.

[waynew]

That's super weird. I'm not familiar with codecov either -- I'm a wee bit confused looking at the coverage report. https://app.codecov.io/gh/aio-libs/aiosmtpd/pull/524/indirect-changes seems to suggest that there are some inderect changes as a result of this PR. If you run with --cov-report term-missing does it show different lines (dis)appearing? :thinking:

[cuu508]

FWIW these indirect coverage changes are not due to code changes in this PR. There are similar coverage changes in previous PRs that did get merged, for example: https://app.codecov.io/gh/aio-libs/aiosmtpd/pull/540/indirect-changes