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

add tests for asyncio #58

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

imnotjames
Copy link
Contributor

@imnotjames imnotjames commented Nov 11, 2023

converts most of the tests from circuitpython's repository into tests that can be run in this repository for each pull request

creates a simple runner that can be used to validate tests

adds a github action which runs the tests for pull requests via the latest circuitpython build

@imnotjames
Copy link
Contributor Author

I was able to run these tests by running the following (with circuitpython cloned)

cd circuitpython
make -C ports/unix variant=coverage -j2
cd ../Adafruit_CircuitPython_asyncio/tests
export MICROPY_MICROPYTHON=../../circuitpython/ports/unix/build-standard/micropython
export MICROPYPATH=../:../../circuitpython/frozen/Adafruit_CircuitPython_Ticks/
python run_tests.py 

This leads to the results:

pass asyncio/asyncio_await_return.py
pass asyncio/asyncio_basic2.py
pass asyncio/asyncio_basic.py
pass asyncio/asyncio_cancel_fair2.py
pass asyncio/asyncio_cancel_fair.py
pass asyncio/asyncio_cancel_self.py
pass asyncio/asyncio_cancel_task.py
pass asyncio/asyncio_cancel_wait_on_finished.py
pass asyncio/asyncio_current_task.py
pass asyncio/asyncio_event_fair.py
pass asyncio/asyncio_event.py
pass asyncio/asyncio_exception.py
pass asyncio/asyncio_fair.py
pass asyncio/asyncio_gather_notimpl.py
pass asyncio/asyncio_gather.py
pass asyncio/asyncio_heaplock.py
pass asyncio/asyncio_lock_cancel.py
pass asyncio/asyncio_lock.py
pass asyncio/asyncio_loop_stop.py
pass asyncio/asyncio_new_event_loop.py
pass asyncio/asyncio_set_exception_handler.py
pass asyncio/asyncio_task_done.py
pass asyncio/asyncio_wait_for_fwd.py
pass asyncio/asyncio_wait_for.py
pass asyncio/asyncio_wait_task.py
--------------------
25 tests performed
25 tests passed

If I instead point it at Python3 and CPython's asyncio I get some failures but that's to be expected because, well, it differs quite a bit in the behavior.

@imnotjames
Copy link
Contributor Author

Also, this build is failing because the expected output files don't have a license but if I add a license then they no longer match my expected outputs..

@imnotjames imnotjames force-pushed the chore/add-tests branch 2 times, most recently from d899635 to 6c21d0f Compare November 12, 2023 02:42
@imnotjames
Copy link
Contributor Author

imnotjames commented Nov 12, 2023

Added a new github actions test runner and modified it to add licensing information because otherwise REUSE whines about the expected values not having licensing info. Sheesh.

@imnotjames imnotjames marked this pull request as ready for review November 12, 2023 03:25
@dhalbert
Copy link
Contributor

What would you think about making circuitpython being a submodule of this module, so you could keep the test code and results in sync with what circuitpython is using? Updating these tests when the tests change is a chore.

@imnotjames
Copy link
Contributor Author

What would you think about making circuitpython being a submodule of this module, so you could keep the test code and results in sync with what circuitpython is using? Updating these tests when the tests change is a chore.

The problem is creating new tests for new features, right? If I am making a new feature like queue, how would I add tests in that scenario?

@dhalbert
Copy link
Contributor

The problem is creating new tests for new features, right? If I am making a new feature like queue, how would I add tests in that scenario?

I would say add your extra tests in the circuitpython repo. We have added a number of extra tests for CircuitPython only, and these are just more.

But this is a chicken and egg problem. The circuitpython CI already runs the tests against a particular commit of this library as a submodule in the circuitpython library. Conversely, you are running the same (+ more tests that you added) against (the latest?) circuitpython. Maybe that should be a particular commit of circuitpython instead of latest.

You could make circuitpython a submodule of this repo, but then you have mutually recursive submodules, which sounds like a bad idea. (Recursive git clone will run forever?!) So maybe the circuitpython commit to use should not be specified as a submodule, but just as a git commit-ish (SHA or tag) stored somewhere to be used by the CI test runner.

(BTW, how are you running the tests manually locally in this repo? Could you document that?)

MicroPython keeps the source of the Python asyncio library in the micropython repo. That makes it easy for MicroPython to keep the tests in sync, but makes it hard to install the library independently. I factored this library out so it could be updated and installed independently.

@imnotjames
Copy link
Contributor Author

imnotjames commented Nov 14, 2023

I would say add your extra tests in the circuitpython repo.

I can do this but it means new features like Queue won't be tested -- or when fixes in wait_for behavior happens it won't get tested. Is that a good tradeoff for a path forward?

You could make circuitpython a submodule of this repo, but then you have mutually recursive submodules, which sounds like a bad idea.

Agreed.

just as a git commit-ish (SHA or tag) stored somewhere to be used by the CI test runner.

I will add the version tags we care about to the CI so it runs against multiple versions, that way it isn't always going against HEAD.

(BTW, how are you running the tests manually locally in this repo? Could you document that?)

I have a built version of micropython which I have been using for it. I'll note in the readme, though I'm not sure the best approach.

cd circuitpython
make -C ports/unix variant=coverage -j2
cd ../Adafruit_CircuitPython_asyncio/tests
export MICROPY_MICROPYTHON=../../circuitpython/ports/unix/build-standard/micropython
export MICROPYPATH=../:../../circuitpython/frozen/Adafruit_CircuitPython_Ticks/
python run_tests.py 

@imnotjames
Copy link
Contributor Author

Should circuitpython's CI be updated to run the tests out of this repo?

@imnotjames
Copy link
Contributor Author

I tried to add 8.2.7 but that seems to have a segmentation fault when running against the tests?

When I checked 8.2.7 in the circuitpython repo it doesn't run the asyncio tests

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

Successfully merging this pull request may close these issues.

2 participants