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

Migrate to pyproject.toml and modernize a little bit the CI #540

Merged
merged 10 commits into from
Oct 29, 2022

Conversation

Terseus
Copy link
Collaborator

@Terseus Terseus commented Oct 28, 2022

This PR arises from a discussion at #503.

Changes summary:

  • Now the CI builds the package and install it from the package before running the tests; this should catch any errors introduced in the package imports or in the package build system.
  • Drops Python 3.6 support; unfortunately I had no choice if we wanted to migrate to pyproject.toml because Python 3.6 doesn't support pyproject.toml, however given that Python 3.6 reached end of line at the beginning of 2022 (see PEP 494) I don't think is a big deal.
  • Add all the current Python versions to CI, up to the new and shiny 3.11.
  • Upgrade CI python-setup action to v4.
  • Introduce the requirements-dev.txt file with the requirements needed for development.
  • Refactor the existing tests from class-based to function-based, mostly to prevent accidental coupling between them.
  • Add TooManyRequestsError and mark all the tests as xfail to prevent the CI failing when Google decides we're not worthy.

@emlazzarin This is the first big step before starting to create a more robust unit test suite that doesn't call always the backend API in every test execution.
The plan was just migrate to pyproject.toml but I got a bit carried away :)

Comments are more than welcome.

Edit: Oh I forgot to add something important, there are some differences in the METADATA file generated:

  • The "Home-Page" directive is not generated anymore, instead an URL with the name "homepage" is generated; in theory this is equivalent (see this discussion at setuptools and this discussion at pypi warehouse) but I'm not 100% sure, maybe we should upload to https://test.pypi.org before merging?
  • The "Author" directive it's not an array of strings but two strings separated by comma, hope it's equivalent to an array; BTW should we assign the dreyco676 to any on the two authors?

This way we can also test:
* if the package can be built
* if the package built can be installed
* if the package imports are correct
The generated packages are equivalent except some minor changes in the
METADATA file:
* The "Home-Page" directive is not supported in pyproject.toml for
  setuptools, the equivalent now is the "homepage" value in the "urls"
  section.
  See pypa/setuptools#3318
* The "Author" directive it's not an array of strings but two strings
  separated by comma, hope it's equivalent to an array.
Python 3.6 doesn't support building packages using pyproject.toml, given
that Python 3.6 reached EOL in 2021-12-23 I think it's safe to drop it.
Function based tests have various advantages over class based tests,
mainly prevent coupling between tests and reduce indentation level.

Also we can stop using `TestCase` and their `self.assertX` and use
instead `assert` and have pytest assert rewrite output, which is much
more helpful.
This way the CI pipeline will not be marked as error when some tests
fails with this error message.

While this introduces the risk that a failing test may pass with a 429
error, the probability is low and with some luck this will not be here
for long.
Copy link
Collaborator

@emlazzarin emlazzarin left a comment

Choose a reason for hiding this comment

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

I wonder if we should merge these into a new dev branch before going into master. We may have an opportunity to clean up a bunch of things at once and make a bigger release.

pyproject.toml Outdated
Comment on lines 20 to 23
"Programming Language :: Python :: 3.3",
"Programming Language :: Python :: 3.4",
"Programming Language :: Python :: 3.5",
"Programming Language :: Python :: 3.6",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm okay with removing these.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, an oversight, I'll update them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I fixed more things in the pyproject.toml:

  • updated the license classifier to match with the LICENSE file.
  • updated the version to match the latest release at pypi (4.8.0).

@Terseus
Copy link
Collaborator Author

Terseus commented Oct 29, 2022

Personally I prefer smaller releases but I'll adapt to your decisions.

However before doing release, please consider doing version tagging (starting with tagging the current v4.8.0) and doing releases; if you want we can start using a CHANGELOG.md file to ease writing the releases changelog, starting in this PR.

My next step is writing unit tests that cover the whole library functionality and doesn't call to the backend at every CI execution.
Once we have this we can start cleaning up the project with more confidence.

The Python versions was very obsolete, and the license classifier wasn't
in sync with the real license described in the `LICENSE` file.
In pypi the latest version is 4.8.0.
@emlazzarin emlazzarin merged commit 712433b into GeneralMills:master Oct 29, 2022
@Terseus Terseus deleted the pyproject-toml branch November 1, 2022 08:36
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