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

Switch test server from pytest-asyncio to subprocess #127

Open
3 tasks done
AlanCoding opened this issue Mar 17, 2025 · 1 comment
Open
3 tasks done

Switch test server from pytest-asyncio to subprocess #127

AlanCoding opened this issue Mar 17, 2025 · 1 comment
Labels
refactoring code build-out and clean-up testing

Comments

@AlanCoding
Copy link
Member

Please confirm the following

  • I agree to follow this project's code of conduct.
  • I have checked the current issues for duplicates.
  • I understand that dispatcher is open source software provided for free and that I might not receive a timely response.

Feature type

New Feature

Feature Summary

Filing partially to replace #81

Taking lessons I learned from #125, I want to describe steps for finally fixing the integration test organization. I'll outline initially in a numbered list:

  1. Remove acontrol_with_reply and everything associated with this type of use. Only synchronous clients will be supported.
  2. Fixtures such as apg_dispatcher will be converted to a synchronous version, using fixture from Add dispatcher/testing/ utilities to help testing #9. This will have to grow a couple of communication utilities and can be merged with the WIP of benchmark tests. In particular, we will probably do socket communication for a select set of asyncio events. Obviously, the subprocess will listen for an exit message.
  3. All asyncio full-server tests will be converted to be synchronous tests
  4. Wherever possible, the server fixtures will be changed to class or module scope to make tests faster for all non-destructive cases

The test requirement pytest-asyncio may still remain for some niche unit tests.

Steps to reproduce

testing work

Current results

No response

Sugested feature result

Integration tests are all synchronous code.

The definition of "integration" will be that it uses a full server in a subprocess.

The full-server fixture will be shared among several tests whenever possible.

As per related issues, utilities to enable this will be in a testing submodule inside the main import path.

Additional information

No response

@AlanCoding AlanCoding added testing refactoring code build-out and clean-up labels Mar 17, 2025
@AlanCoding
Copy link
Member Author

I didn't fully wrap this up with the "why?"

Because, writing async tests mean that we write a duplicate version of all client functions in asyncio code. While this might eventually be a feature request, we have no immediate need for it in any non-testing sense. Even if we intend to do this, the current protocol for Broker classes are not fully coherent, as writing the socket client has revealed to me. In the pg-notify broker, we use the same methods for the client control-with-reply as for the server. This has an unwritten assumption that the broker behaves symmetrically (client listening has the same form as server-side listening). This is not true for the socket broker, and many other types of brokers.

Thus, we abandon our partially-supported methods for async clients. Doing this means that we must abandon our async integration tests. The obvious path forward is to rewrite them to be synchronous for the test itself, which is this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring code build-out and clean-up testing
Projects
None yet
Development

No branches or pull requests

1 participant