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

python3Packages.mocket: Resolve test failures #216511

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hesiod
Copy link
Contributor

@hesiod hesiod commented Feb 15, 2023

Description of changes

Add pytest-asyncio in order to allow async tests to run. They were previously silently skipped with a warning.

Disable three more tests (test_http*_session) that depend on Internet connectivity and thus fail in sandboxed builds.

This should some Hydra failures, e.g. this one: https://hydra.nixos.org/build/208832590

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Comment on lines 77 to 79
"test_http_session"
"test_https_session"
"test_httprettish_session"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set __darwinAllowLocalNetworking = true; instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I didn't know about that! However, I'm not sure if it's even a Darwin-specific issue anymore -see below

@hesiod
Copy link
Contributor Author

hesiod commented Feb 15, 2023

The tests also fail on x86_64-linux for me (even with the sandbox disabled).
At this point I think it might actually be due to changes to asyncio.get_event_loop(), evident in this warning:

tests/main/test_http_aiohttp.py::AioHttpEntryTestCase::test_http_session
  /build/mocket-3.10.9/tests/main/test_http_aiohttp.py:35: DeprecationWarning: There is no current event loop
    loop = asyncio.get_event_loop()

(printed a couple of times)

That would mean it's simply an upstream Python 3.11 compatibility issue.
Running nixpkgs-review reveals similar failures in reverse dependencies of mocket.

@hesiod
Copy link
Contributor Author

hesiod commented Feb 15, 2023

Result of nixpkgs-review pr 216511 run on x86_64-linux 1

2 packages marked as broken and skipped:
  • python310Packages.pytest-ansible
  • python311Packages.pytest-ansible
11 packages failed to build:
  • python311Packages.ansible
  • python311Packages.ansible-compat
  • python311Packages.ansible-core
  • python311Packages.ansible-kernel
  • python311Packages.ansible-later
  • python311Packages.ansible-lint
  • python311Packages.ansible-runner
  • python311Packages.geoip2
  • python311Packages.parsedmarc
  • python311Packages.sopel
  • python311Packages.ttp
16 packages built:
  • ansible (ansible_2_14 ,python310Packages.ansible-core)
  • ansible-later (python310Packages.ansible-later)
  • ansible-lint (python310Packages.ansible-lint)
  • ansible_2_12
  • ansible_2_13
  • kargo
  • parsedmarc (python310Packages.parsedmarc)
  • python310Packages.ansible
  • python310Packages.ansible-compat
  • python310Packages.ansible-kernel
  • python310Packages.ansible-runner
  • python310Packages.geoip2
  • python310Packages.mocket
  • python310Packages.sopel
  • ttp (python310Packages.ttp)
  • python311Packages.mocket

Edit:

python311Packages.geoip2 fails with very similiar errors to the original mocket errors. And the relevant geoip2 tests use mocket.

geoip2 test output (excerpt):
/nix/store/3jxwq9ln4iizm8b01bcnfzn98ivyrhwf-python3.11-aiohttp-3.8.3/lib/python3.11/site-packages/aiohttp/client.py:572: ClientOSError
=========================== short test summary info ============================
FAILED tests/webservice_test.py::TestAsyncClient::test_200_error - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_300_error - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_500_error - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_account_id_required - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_account_id_unkown - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_auth_invalid - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_bad_body_error - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_city_ok - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_country_ok - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_insights_ok - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_ip_address_not_found - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_ip_address_required - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_ip_address_reserved - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_license_key_required - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_me - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_no_body_error - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_out_of_queries_error - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_permission_required - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_request - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_unknown_error - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_user_id_required - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_user_id_unkown - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
FAILED tests/webservice_test.py::TestAsyncClient::test_weird_body_error - aiohttp.client_exceptions.ClientOSError: Cannot write to closing transport
======================== 23 failed, 128 passed in 1.88s ========================

Add pytest-asyncio in order to allow async tests to run. They were
previously silently skipped with a warning.

Disable three more tests (test_http*_session) that depend on Internet
connectivity and thus fail in sandboxed builds.
@hesiod
Copy link
Contributor Author

hesiod commented Feb 15, 2023

After more fiddling, I think the root issue is simply some Python 3.11 incompatibility in the test setup of mocket (althought I'm not sure what the cause is). Anyways, in the newest version of the commits I'm conditionally disabling certain tests on Python >= 3.11 in mocket and geoip2 - the latter has similar 3.11 issues that weren't uncovered previously because mocket didn't build.

@ofborg ofborg bot requested a review from mweinelt February 15, 2023 20:30
@hesiod hesiod marked this pull request as ready for review February 17, 2023 13:35
@davidak
Copy link
Member

davidak commented Feb 17, 2023

Result of nixpkgs-review pr 216511 run on x86_64-linux 1

2 packages marked as broken and skipped:
  • python310Packages.pytest-ansible
  • python311Packages.pytest-ansible
1 package failed to build:
  • python311Packages.sopel
26 packages built:
  • ansible (ansible_2_14 ,python310Packages.ansible-core)
  • ansible-later (python310Packages.ansible-later)
  • ansible-lint (python310Packages.ansible-lint)
  • ansible_2_12
  • ansible_2_13
  • kargo
  • parsedmarc (python310Packages.parsedmarc)
  • python310Packages.ansible
  • python310Packages.ansible-compat
  • python310Packages.ansible-kernel
  • python310Packages.ansible-runner
  • python310Packages.geoip2
  • python310Packages.mocket
  • python310Packages.sopel
  • ttp (python310Packages.ttp)
  • python311Packages.ansible
  • python311Packages.ansible-compat
  • python311Packages.ansible-core
  • python311Packages.ansible-kernel
  • python311Packages.ansible-later
  • python311Packages.ansible-lint
  • python311Packages.ansible-runner
  • python311Packages.geoip2
  • python311Packages.mocket
  • python311Packages.parsedmarc
  • python311Packages.ttp

sopel issue maybe unrelated:

error: builder for '/nix/store/sj9krg0mdd8r98hkiadlrr51n96rhw37-python3.11-sopel-7.1.9.drv' failed with exit code 1;
       last 10 log lines:
       > FAILED test/test_bot.py::test_register_plugin - ValueError: invalid file open mode 'U'
       > FAILED test/test_bot.py::test_register_unregister_plugin - ValueError: invalid file open mode 'U'
       > FAILED test/test_loader.py::test_is_limitable - ValueError: invalid file open mode 'U'
       > FAILED test/test_loader.py::test_is_triggerable - ValueError: invalid file open mode 'U'
       > FAILED test/test_loader.py::test_is_url_callback - ValueError: invalid file open mode 'U'
       > FAILED test/test_loader.py::test_clean_module - ValueError: invalid file open mode 'U'
       > FAILED test/test_loader.py::test_clean_module_idempotency - ValueError: invalid file open mode 'U'
       > FAILED test/test_plugins.py::test_plugin_load_pymod - ValueError: invalid file open mode 'U'
       > ======================== 8 failed, 317 passed in 2.01s =========================
       > /nix/store/b09v23lirgvci3wzszh22mbkdfj0h0yq-stdenv-linux/setup: line 1582: pop_var_context: head of shell_variables not a function context
       For full logs, run 'nix log /nix/store/sj9krg0mdd8r98hkiadlrr51n96rhw37-python3.11-sopel-7.1.9.drv'.

Copy link
Member

@davidak davidak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good

Build successful

@fabaff
Copy link
Member

fabaff commented Feb 20, 2023

#217295 contains the update of mocket to 3.11.0 which solved the build failure of mocket itself.

@@ -30,6 +30,8 @@ buildPythonPackage rec {
pytestCheckHook
];

disabledTests = lib.optional (pythonAtLeast "3.11") "TestAsyncClient";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me there was also a failure on Python 3.10, see e2be015

@figsoda figsoda added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Mar 5, 2023
@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: python 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants