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

Run normal pytest tests in pyodide #62

Merged
merged 56 commits into from
Dec 29, 2022

Conversation

joemarshall
Copy link
Collaborator

This is an extra command line parameter which tells the pytest plugin that you want to run a normal pytest set of tests (i.e. non-pyodide aware, just from any package) but run each test in pyodide.

This means that if you have the pytest plugin installed and working, you can run:

# first make a wheel from the current project in dist subfolder, this auto-copies that
pyodide build 

# this autocopies <test_folder> to pyodide and then runs each collected test in pyodide. 
pytest --run-in-pyodide --runtime=chrome test_folder

and you should see almost exactly the output you would see if you ran
pytest test_folder normally, but with the tests themselves running in pyodide on a browser.

@hoodmane
Copy link
Member

Thanks @joemarshall this sounds great. How do you deal with dependencies? Maybe we could pass a requirements.txt file and a wheels directory where the wheels directory has to include a wheel for each requirement missing from the Pyodide repodata.json file?

@hoodmane
Copy link
Member

Or maybe we could make a virtual environment and ship the site_packages directory of the environment into the browser, and install the dependencies with pip. That's more similar to my command line runner route.

@joemarshall
Copy link
Collaborator Author

Right now it just installs any wheels in the dist directory. So if you have deps that micropip won't install automatically, you can
put them in that folder as wheels.

Thinking about it though, I'm not sure how dependency ordering will work - if micropip installs dependencies when you install a wheel, then it in theory should just be a matter of making the wheel require the correct deps. But, a question - if you give micropip a list of wheels, does it correctly resolve them no matter what order they're in? i.e. if I give it main.whl and dep1.whl
in a single install command, will it download just main.whl first and then fail to find dep1, or does it download all given wheels and then try resolving dependencies?

@hoodmane
Copy link
Member

But, a question - if you give micropip a list of wheels, does it correctly resolve them no matter what order they're in?

I'm not sure...

@joemarshall
Copy link
Collaborator Author

I just had a think about this - I think the right thing to do here is do a bit of work on pyodide build to enable it to build a project and collect (and build if needed) all deps into the dist folder. That way once you have the project working with pyodide build, you both have all required dependencies for running it in pyodide or jupyterlite easy to hand, and also for running pytest on it. Needs a little thought as to how to specify what doesn't need building (e.g. if building things which support jupyter, everything is a hell of a lot easier if you tell it to ignore various jupyter deps which themselves have complex dependency trees, which are implemented by jupyterlite anyway)

I've got to make roughly that functionality work anyway for what I'm working on, and the client on this is keen for things to upstream where possible as opposed to having to maintain complex build scripts, so it could be a win win.

@joemarshall
Copy link
Collaborator Author

Oh, I realized why this wouldn't pass tests - I had a bunch of broken testing stuff left in github actions. Sorry, fixed now.

@hoodmane I think subject to the dependency resolution stuff going into pyodide build, this is hopefully ready to go

@joemarshall joemarshall mentioned this pull request Dec 6, 2022
3 tasks
@joemarshall
Copy link
Collaborator Author

@ryanking13 Actually splitting it would be a bit fiddly, but I have split the key logic in the code into two files - run_tests_inside_pyodide.py which calls tests inside pyodide, and copy_files_to_pyodide.py which does the copying stuff. I also added a test decorator for copying files, which allows you to use the copy files functionality in other tests.

I didn't yet add a command line for copying files, but I think that would be a pretty trivial addition, I just ran out of time. If you're okay, I can merge this version now it tests okay, and add the copying files command line later?

Copy link
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

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

Thanks, @joemarshall. I have several comments, but it generally looks like a nice improvement.

add the copying files command line later?

Yes, of course.

pytest_pyodide/hook.py Show resolved Hide resolved
pytest_pyodide/hook.py Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
pytest_pyodide/copy_files_to_pyodide.py Show resolved Hide resolved
pytest_pyodide/copy_files_to_pyodide.py Show resolved Hide resolved
pytest_pyodide/hook.py Outdated Show resolved Hide resolved
pytest_pyodide/hook.py Show resolved Hide resolved
pytest_pyodide/run_tests_inside_pyodide.py Outdated Show resolved Hide resolved
tests/test_copy_files.py Show resolved Hide resolved
old_dir = Path.cwd()
try:
os.chdir(td)
run([sys.executable, "-m", "pip", "download", "pyodide_http"])
Copy link
Member

Choose a reason for hiding this comment

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

Maybe let's embed test wheel file inside the repository instead of downloading it every time.

joemarshall and others added 4 commits December 27, 2022 23:16
Co-authored-by: Gyeongjae Choi <def6488@gmail.com>
Co-authored-by: Gyeongjae Choi <def6488@gmail.com>
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.

4 participants