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 script to test packaging #1545

Merged
merged 2 commits into from
Apr 13, 2020
Merged

Conversation

kclowes
Copy link
Collaborator

@kclowes kclowes commented Dec 6, 2019

What was wrong?

We had a release (5.3.1) that was missing a dependency. Unfortunately, that wasn't caught until after the release.

Related to Issue #1540

How was it fixed?

I stole a script from trinity that installs the release to a temporary virtualenv for testing.

Todo:

Cute Animal Picture

image

@kclowes
Copy link
Collaborator Author

kclowes commented Dec 6, 2019

@pipermerriam It looks like this script only builds from the wheel that is already in dist/. Does the Trinity team usually do a new local build and then run this script, or what is your preferred flow?

@pipermerriam
Copy link
Member

cc @carver did you setup the wheel tests for trinity?

@veox
Copy link
Contributor

veox commented Dec 19, 2019

@kclowes The dist is built as part of the *-wheel-cli CI jobs; here's the linked lines:

commands=
    /bin/rm -rf build dist
    python setup.py sdist bdist_wheel
    /bin/bash -c 'pip install --upgrade "$(ls dist/*.whl)""[p2p,trinity]" --progress-bar off'
    pytest {posargs:tests/integration/ -k 'trinity_cli' --log-cli-level 0}

(As you can see, it also runs a small selection of integration tests. Something like python -c 'from web3.auto import w3' could be used as a placeholder.)

It was first added in PR ethereum/trinity#106, and later modified.

@kclowes
Copy link
Collaborator Author

kclowes commented Dec 19, 2019

Aha! Thanks for looking into that @veox!

@kclowes kclowes changed the title Add script to test packaging [WIP] Add script to test packaging Dec 19, 2019
@kclowes kclowes changed the title [WIP] Add script to test packaging Add script to test packaging Jan 10, 2020
@kclowes kclowes force-pushed the add-release-test branch 2 times, most recently from f4420ef to 26ec05a Compare January 10, 2020 17:25
Copy link
Member

@pipermerriam pipermerriam left a comment

Choose a reason for hiding this comment

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

Looks like this file might not have type hints enforced. I would recommend doing that ahead of this (or as part of it) so that you can ensure that all new code to the repo has type hints.

Copy link
Member

@pipermerriam pipermerriam left a comment

Choose a reason for hiding this comment

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

Looks 👍

I'm not seeing anywhere that the test_package.py script is being invoked in CI am I missing something?

@kclowes
Copy link
Collaborator Author

kclowes commented Apr 13, 2020

Nope, good catch. Instead of adding to CI, I added to the Makefile and the README similar to what Trinity has going on. There is a smoke test in CI that makes sure that the wheel builds and that we can import web3 though.

@kclowes kclowes merged commit a0a1763 into ethereum:master Apr 13, 2020
@kclowes kclowes deleted the add-release-test branch April 13, 2020 21:04
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.

3 participants