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

Update dev requirements #688

Merged
merged 8 commits into from
Dec 9, 2022
Merged

Update dev requirements #688

merged 8 commits into from
Dec 9, 2022

Conversation

willkg
Copy link
Member

@willkg willkg commented Dec 8, 2022

This updates dev requirements. While doing that, I split out flake8 into its own world because its dependencies conflict with other dependencies.

This updates dev requirements. While doing that, I split out flake8 into
its own world because its dependencies conflict with other dependencies.
This fixes the lint workflow to run the tox environments which build the
correct environment and then runs the script inside it.
This redoes how linting and testing work in CI. This uses tox-gh-actions
to run tox in the right python environments. It switches the linting
items to the test workflow because they're defined as tox environments.
@willkg
Copy link
Member Author

willkg commented Dec 8, 2022

I updated the main branch checks dropping the lint workflow items and adding the Python 3.11 items.

@willkg
Copy link
Member Author

willkg commented Dec 8, 2022

While I'm redoing workflows, I think I'm going to make everything tox-centric in this PR.

This fixes the issue where a bunch of tox-defined environments weren't
being run in CI, so there was a whole bunch of testing that wasn't
happening on pull requests.

Now everything is tox-based and CI runs all the tox environments by
Python environment using tox-gh-actions.
@willkg
Copy link
Member Author

willkg commented Dec 8, 2022

This looks a lot more correct now:

Run tox
GLOB sdist-make: /home/runner/work/bleach/bleach/setup.py
py39 create: /home/runner/work/bleach/bleach/.tox/py39
tox: py39
py39-tinycss2 create: /home/runner/work/bleach/bleach/.tox/py39-tinycss2
tox: py39-tinycss2
py39-build-no-lang create: /home/runner/work/bleach/bleach/.tox/py39-build-no-lang
tox: py39-build-no-lang
py39-docs create: /home/runner/work/bleach/bleach/.tox/py39-docs
tox: py39-docs
py39-format-check create: /home/runner/work/bleach/bleach/.tox/py39-format-check
tox: py39-format-check
py39-lint create: /home/runner/work/bleach/bleach/.tox/py39-lint
tox: py39-lint
py39-vendorverify create: /home/runner/work/bleach/bleach/.tox/py39-vendorverify
tox: py39-vendorverify

shell: bash
run: ./scripts/run_tests.sh
run: tox
Copy link
Member Author

Choose a reason for hiding this comment

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

Switching it to be tox-based where tox-gh-action will run the various tox environments depending on the Python version. Now we're running all the tox environments. Previously, we were only running the basic ones and skipping the tinycss2 ones and the build-no-lang ones in CI.


include docs/conf.py
include docs/Makefile
include docs/requirements.txt

include scripts/*

recursive-include bleach *.py *.json *.rst *.sh *.txt INSTALLER METADATA RECORD WHEEL LICENSE REQUESTED *.SHA256SUM
recursive-include bleach *.py *.rst *.sh *.txt INSTALLER METADATA RECORD WHEEL LICENSE REQUESTED *.SHA256SUM
Copy link
Member Author

Choose a reason for hiding this comment

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

There were no json files in the bleach/ directory, so I removed this bit.

rm -rf docs/_build/*
rm -rf .eggs
find . -name __pycache__ | xargs rm -rf
find . -name '*.pyc' | xargs rm -rf
Copy link
Member Author

Choose a reason for hiding this comment

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

The docs talk about a Makefile, but there wasn't one in the repo. This adds one.

@@ -85,8 +85,7 @@ Release process
exit with ``/tmp/vendor-test exists. Please remove.`` and the exit
code should be zero)::

$ ./scripts/run_tests.sh vendorverify
$ ./scripts/run_tests.sh vendorverify
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure why this has to be run twice. I think it was an error.

@@ -102,21 +101,21 @@ Release process

9. Generate distribution files::

$ python setup.py sdist bdist_wheel
$ python -m build
Copy link
Member Author

Choose a reason for hiding this comment

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

We want to use the build module now. https://pypi.org/project/build/

@@ -0,0 +1,2 @@
# Requirements for running flake8
flake8==6.0.0
Copy link
Member Author

Choose a reason for hiding this comment

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

flake8 has dependency conflicts with other packages, so I moved it into its own environment.

@@ -28,13 +28,6 @@ case "${MODE}" in
format-check)
Copy link
Member Author

Choose a reason for hiding this comment

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

Oops--I meant to remove this file entirely. We don't need it anymore.

py39-docs
py39-format-check
py39-lint
py39-vendorverify
Copy link
Member Author

Choose a reason for hiding this comment

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

Adding the py39 encodes which Python version to run it in.

[testenv:py39-lint]
changedir = {toxinidir}
deps = -rrequirements-flake8.txt
platform = linux
Copy link
Member Author

Choose a reason for hiding this comment

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

Adding platform = linux here means this will only run when the platform is linux. This prevents it from running on windows and macos--we don't need to run these utility environments on those environments.

Copy link
Member Author

Choose a reason for hiding this comment

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

This probably also means that devs who are using Windows and macOS can't run these environments. I'm the only dev at the moment and I'm using Linux this week, so I'm going to ignore this for now, but I'll write it up as an issue.

Copy link
Member Author

@willkg willkg Dec 8, 2022

Choose a reason for hiding this comment

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

[grumble] I should fix it now.

Copy link
Member Author

Choose a reason for hiding this comment

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

For things like vendorverify and docs, we want to run them in CI only on Linux, but when not in CI on whatever platform. I can't figure out a way to do that that doesn't involve writing a bash script or something else that's platform-dependent, so I'm going to skip it for now.

tox.ini Outdated
3.9: py39
3.10: py310
3.11: py311
py3: pypy-3.8
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this isn't working correctly.

Run actions/setup-python@v4
Successfully set up PyPy 7.3.9
 with Python (3.8.13)

It should be installing PyPy 3.8 not PyPy 7.3.9 using Python 3.8.13. Either that or I'm misreading the output.

@willkg
Copy link
Member Author

willkg commented Dec 8, 2022

That fixes running CI with pypy.

Run tox
GLOB sdist-make: /home/runner/work/bleach/bleach/setup.py
pypy3 create: /home/runner/work/bleach/bleach/.tox/pypy3
tox: pypy3
pypy3-tinycss2 create: /home/runner/work/bleach/bleach/.tox/pypy3-tinycss2
tox: pypy3-tinycss2

Now to re-fix tox utility environments so anyone can run them, but they only run in Linux in CI.

@willkg
Copy link
Member Author

willkg commented Dec 9, 2022

I changed the name of the pypy jobs, so I had to update branch protections for main accordingly.

@willkg willkg merged commit a0cadd9 into mozilla:main Dec 9, 2022
@willkg willkg deleted the update-dev-deps branch December 23, 2022 14:01
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.

1 participant