Skip to content
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

Draft: Run the pynntp tests in the build GitHub Action #313

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Oct 1, 2024

Help needed: After ci/install and ci/test should the inn server be running so that clients can access it? If so, what should the hostname be? If not, could it be run in a GitHub Actions service (or other background job) like inn-docker does?

Based on https://github.com/greenbender/inn-docker/actions running these pytests should add ~30 seconds but give contributors rapid visibility to breakage with the Python client.


% pytest

============================= test session starts ==============================
platform linux -- Python 3.12.6, pytest-8.3.3, pluggy-1.5.0
rootdir: /home/runner/work/inn/inn
configfile: pyproject.toml
collected 54 items
tests/integration/inn2/test_inn2.py FFxxFFFFFFFFFFFF                     [ 29%]
tests/test_headerdict.py ..........                                      [ 48%]
tests/test_utils.py ..xxx...xxx....xxx.....x..                           [ 96%]
tests/test_yenc.py ..                                                    [100%]
=================================== FAILURES ===================================
[ ... ]
>       with nntp.NNTPClient("localhost") as nntp_client:
[ ... ]
/opt/hostedtoolcache/Python/3.12.6/x64/lib/python3.12/socket.py:850: ConnectionRefusedError
[ ... ]
=========================== short test summary info ============================
FAILED tests/integration/inn2/test_inn2.py::test_nntp_client - ConnectionRefusedError: [Errno 111] Connection refused
FAILED tests/integration/inn2/test_inn2.py::test_context_manager - ConnectionRefusedError: [Errno 111] Connection refused
FAILED tests/integration/inn2/test_inn2.py::test_nntp_client_without_ssl - ConnectionRefusedError: [Errno 111] Connection refused
FAILED tests/integration/inn2/test_inn2.py::test_nntp_client_with_ssl - ConnectionRefusedError: [Errno 111] Connection refused
FAILED tests/integration/inn2/test_inn2.py::test_nntp_client_with_ssl_starttls - ConnectionRefusedError: [Errno 111] Connection refused
FAILED tests/integration/inn2/test_inn2.py::test_post[local.general] - ConnectionRefusedError: [Errno 111] Connection refused
FAILED tests/integration/inn2/test_inn2.py::test_post[local.test] - ConnectionRefusedError: [Errno 111] Connection refused
FAILED tests/integration/inn2/test_inn2.py::test_post[junk] - ConnectionRefusedError: [Errno 111] Connection refused
FAILED tests/integration/inn2/test_inn2.py::test_list_active[local.general] - ConnectionRefusedError: [Errno 111] Connection refused
FAILED tests/integration/inn2/test_inn2.py::test_list_active[local.test] - ConnectionRefusedError: [Errno 111] Connection refused
FAILED tests/integration/inn2/test_inn2.py::test_list_active[junk] - ConnectionRefusedError: [Errno 111] Connection refused
FAILED tests/integration/inn2/test_inn2.py::test_article[local.general] - ConnectionRefusedError: [Errno 111] Connection refused
FAILED tests/integration/inn2/test_inn2.py::test_article[local.test] - ConnectionRefusedError: [Errno 111] Connection refused
FAILED tests/integration/inn2/test_inn2.py::test_article[junk] - ConnectionRefusedError: [Errno 111] Connection refused
================== 14 failed, 28 passed, 12 xfailed in 1.32s ===================

@Julien-Elie
Copy link
Contributor

Yes, the news server needs running; otherwise the client cannot connect.
It would indeed be useful to have functional tests like what pynntp does. We currently mostly have unitary testing.
You can use whatever hostname works for the test, and also in a docker if need be.

As the test runs for each commit, the GitHub Actions should be stable and not cause the commit to be marked with a build or test failure that is not caused by the commit.

@Julien-Elie
Copy link
Contributor

Seems like pynntp fails because there's no running news server. Note that, contrary to an installation with apt-get, the CI just builds INN and runs the test suite. It does not install INN, neither launches its programs.
Maybe what you want is to install INN before running pynntp?

You may need rebuilding INN with different options as you're not root on GitHub Actions. I don't know which user/group is used, but here is the idea:

make distclean
./configure CC="${COMPILER:-gcc}" --prefix=/home/runner/work/inn/inn-install --with-news-user=USER --with-news-group=GROUP --with-perl --with-python --with-sasl --with-zlib --with-canlock --with-openssl
make
make install
INN_HOSTNAME=inn.github-action /home/runner/work/inn/inn-install/bin/rc.news start

I don't know whether GitHub accepts that a program binds to the privileged 119 port. If that's not the case, you may use for instance port 1119.

Based on https://github.com/greenbender/inn-docker/actions running these pytests should add ~30 seconds but give contributors rapid visibility to breakage with the Python client.
@cclauss
Copy link
Contributor Author

cclauss commented Oct 7, 2024

% INN_HOSTNAME=inn.github-action $RUNNER_WORKSPACE/inn-install/bin/rc.news start

Starting innd.
innd: must be run as news group

@Julien-Elie
Copy link
Contributor

Do you happen to have made in progress on the integration of the pynntp test suite?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants