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

Test file re-organization #81

Closed
3 tasks done
AlanCoding opened this issue Feb 18, 2025 · 5 comments
Closed
3 tasks done

Test file re-organization #81

AlanCoding opened this issue Feb 18, 2025 · 5 comments
Labels
question Further information is requested refactoring code build-out and clean-up

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

Enhancement to Existing Feature

Feature Summary

This proposes a change to organization of files inside of tests/ folder in this repo.

My main reason for doing this is because of painful limitations of pytest-asyncio that I wasn't aware when I started. Most of my time writing tests have been debugging the test event loop, as opposed to any meaningful debugging of the service.

  • tests/integration/ --> tests/asyncio/ these are tests using pytest-asyncio
  • tests/integration/ --> a new suite that runs the server in a subprocess, which is connected to what I was saying in Add dispatcher/testing/ utilities to help testing #9, and also the method I expect to be used in the DAB app
  • tests/benchmark/ --> patch at Benchmark tests #55, this also runs the server in a subprocess, and hopefully we can merge that with the general fixture. There might be some more complex event exchange through pipes to do this.
  • tests/data/ --> exists, has stuff that tests target
  • tests/unit/ --> exists, tests that don't need postgres

Ping @Alex-Izquierdo FYI, I have another file moving issue #68

Steps to reproduce

N/A

Current results

No response

Sugested feature result

N/A

Additional information

No response

@AlanCoding AlanCoding added the refactoring code build-out and clean-up label Feb 18, 2025
@Alex-Izquierdo
Copy link
Collaborator

Why not simply use tests/integration/sync and tests/integration/async, since they will be integration tests anyway? I don't fully understand the issue at this point. However, if the directory structure is a limitation, an alternative could be tests/integration-sync and tests/integration-async.
My main point is to avoid mixing the test scope with the technology in the directory organization, as they are different concerns.
Also I understand that eventually we would need as well asyncio tests for unit tests, am I wrong?

@AlanCoding
Copy link
Member Author

Oh I assumed asyncio unit tests would just go in tests/unit/. Might be some in there now. Guess I never thought about it.

tests/integration/async would wind up with a lot of nesting.

@AlanCoding
Copy link
Member Author

I also consider the pytest-asyncio tests "inferior", meaning "not true integration" tests. In my head, a subprocess has essentially the full mechanics of a backgrounded dispatcher in-place. The syncio are then "between" unit and integration on the spectrum of how integrated they are.

@AlanCoding
Copy link
Member Author

Based on actual experience, I want to go in a different direction, introduce:

  • tests/session/

The idea is to have tests/session/conftest.py that has @pytest.fixture(scope="module"). Or maybe in each file in that for different configs.

Then each test uses the fixture that gives an already-running server. It doesn't start the server for that test. The server runs for the life of all the tests within that module. It still uses postgres and a full server like other tests. This could be asyncio or synchronous, or vary by module.

There are many, many, tests that could be ran in this context. Almost all. These tests could not leave jobs running doing sleeps. One way around that is that we could issue a cancel command at the end of the test.

Doing this could notably improve the experience of running tests. Having the scope separation based on folder would help to accomplish this IMO.

@AlanCoding AlanCoding added the question Further information is requested label Feb 27, 2025
@AlanCoding
Copy link
Member Author

Replacing another issue, because this doesn't cut to the true core of what's awkward about writing tests here, and bigger pain points have become clear.

@AlanCoding AlanCoding reopened this Mar 17, 2025
@AlanCoding AlanCoding closed this as not planned Won't fix, can't repro, duplicate, stale Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested refactoring code build-out and clean-up
Projects
None yet
Development

No branches or pull requests

2 participants