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 test suite to use pytest, remove related in setup.py #771

Merged
merged 11 commits into from
Jul 29, 2024

Conversation

kwongtn
Copy link
Contributor

@kwongtn kwongtn commented Jul 29, 2024

Setuptools removes use of setuptools.command.test
This PR serves as a quick temporary fix.

Refer:

@wongjiahau
Copy link

Running poetry add git+https://github.com/kwongtn/django-celery-beat#restrict-setuptools-to-max-v71 still fails with the same error

@cclauss
Copy link
Contributor

cclauss commented Jul 29, 2024

Nice initiative!! This is still happening on our GitHub Actions Py3.12 tests...

ModuleNotFoundError: No module named 'setuptools.command.test'

setuptools changelog: https://setuptools.pypa.io/en/stable/history.html#v72-0-0

In this repo:

$(PYTHON) setup.py test

@cclauss cclauss changed the title Fix setuptools to <v72 Limit setuptools to <v72 Jul 29, 2024
@kwongtn
Copy link
Contributor Author

kwongtn commented Jul 29, 2024

If anyone wants to take over and continue fixing, please do so..
I'm out of ideas 😥

@paretosti
Copy link

paretosti commented Jul 29, 2024

@wongjiahau

Running poetry add git+https://github.com/kwongtn/django-celery-beat#restrict-setuptools-to-max-v71 still fails with the same error

Is this the right way to install a branch? Is it not

poetry add git+https://github.com/kwongtn/django-celery-beat/restrict-setuptools-to-max-v71

Edit: Sorry I think i must be missing something...

setup.py Outdated Show resolved Hide resolved
@shijl0925
Copy link

pip3 install django-celery-beat@git+https://github.com/kwongtn/django-celery-beat.git@fix/restrict-setuptools-to-max-v71

Collecting django-celery-beat@ git+https://github.com/kwongtn/django-celery-beat.git@fix/restrict-setuptools-to-max-v71
Cloning https://github.com/kwongtn/django-celery-beat.git (to revision fix/restrict-setuptools-to-max-v71) to /tmp/pip-install-otwx_qhh/django-celery-beat_a2925745ef8d482b967cd8ec9c642a08
Running command git clone --filter=blob:none --quiet https://github.com/kwongtn/django-celery-beat.git /tmp/pip-install-otwx_qhh/django-celery-beat_a2925745ef8d482b967cd8ec9c642a08
Running command git checkout -b fix/restrict-setuptools-to-max-v71 --track origin/fix/restrict-setuptools-to-max-v71
Switched to a new branch 'fix/restrict-setuptools-to-max-v71'
branch 'fix/restrict-setuptools-to-max-v71' set up to track 'origin/fix/restrict-setuptools-to-max-v71'.
Resolved https://github.com/kwongtn/django-celery-beat.git to commit 5a21c38
Installing build dependencies ... done
Getting requirements to build wheel ... error
error: subprocess-exited-with-error

× Getting requirements to build wheel did not run successfully.
│ exit code: 1
╰─> [20 lines of output]
Traceback (most recent call last):
File "/usr/local/lib/python3.11/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in
main()
File "/usr/local/lib/python3.11/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main
json_out['return_val'] = hook(**hook_input['kwargs'])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/pip/_vendor/pyproject_hooks/in_process/in_process.py", line 118, in get_requires_for_build_wheel
return hook(config_settings)
^^^^^^^^^^^^^^^^^^^^^
File "/tmp/pip-build-env-i22alft
/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 327, in get_requires_for_build_wheel
return self.get_build_requires(config_settings, requirements=[])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/tmp/pip-build-env-i22alft
/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 297, in get_build_requires
self.run_setup()
File "/tmp/pip-build-env-i22alft
/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 497, in run_setup
super().run_setup(setup_script=setup_script)
File "/tmp/pip-build-env-i22alft
/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 313, in run_setup
exec(code, locals())
File "", line 123, in
NameError: name 'pytest' is not defined. Did you mean: 'bytes'?
[end of output]

note: This error originates from a subprocess, and is likely not a problem with pip.

the code should be removed?

cmdclass={'test': pytest}

@kwongtn
Copy link
Contributor Author

kwongtn commented Jul 29, 2024

@shijl0925 Yup I think you're right -- removed it and it works nicely now

Not sure how it will affect the test in this repo tho 😅
But I replaced it for pytest to be called directly from terminal instead of being invoked by setup.py

I'll be locking my prod build to 65d5743 , but @cclauss feel free to make required edits

@kwongtn kwongtn marked this pull request as ready for review July 29, 2024 06:02
@kwongtn kwongtn changed the title Limit setuptools to <v72 Update test suite to use pytest, remove related in setup.py Jul 29, 2024
.github/workflows/test.yml Outdated Show resolved Hide resolved
@@ -139,7 +139,7 @@ test-all: clean-pyc
$(TOX)

test:
$(PYTHON) setup.py test
$(PYTHON) -m $(PYTEST)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does make test work on your machine?

Copy link
Contributor Author

@kwongtn kwongtn Jul 29, 2024

Choose a reason for hiding this comment

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

Unfortunately no, but tox does though

Although t/unit/test_schedulers.py::test_modeladmin_PeriodicTaskAdmin::test_run_task fail 😅

requirements/pkgutils.txt Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
@cclauss
Copy link
Contributor

cclauss commented Jul 29, 2024

Please try make test on your computer.

Copy link
Contributor

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

@kwongtn. Awesome work! Thanks for doing this.

@Nusnus @auvipy We will need a new release to PyPI to cover the breaking change in setuptools.
Also see:

@cclauss cclauss merged commit a50011d into celery:main Jul 29, 2024
21 checks passed
daniilr pushed a commit to daniilr/django-celery-beat that referenced this pull request Jul 29, 2024
* Fix setuptools to <v72

* Makefile: Use pytest instead of setup.py test

* Attempt to use pytest directly

* Add python -m

* Update setup.py

Co-authored-by: Christian Clauss <cclauss@me.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add pytest installation to workflow

* Remove pytest from setup.py

* Unlock setuptools

* Remove tests_require

* Move pytest to tox.ini

---------

Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
(cherry picked from commit a50011d)
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.

6 participants