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

chore: Simplify tox, bump dependencies #5607

Merged
merged 5 commits into from
Aug 14, 2024

Conversation

holmanb
Copy link
Member

@holmanb holmanb commented Aug 12, 2024

Additional Context

  • bump dependency versions (black, mypy, etc)
  • fix mypy issues for newer versions
  • since python3.7 is no longer supported by pytest, make a separate tox environment and CI job for the old version
  • switch to Ubuntu 22.04 where needed for improved pip dependency resolution

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
When referencing a command from another environment, it will cause
errors when the other environment already exists. Fix it by avoiding
indirection in environment command definitions.

Additionally, simplify envoronment dependency management by defining two
lists of dependencies: a default one with pinned versions for all
environments, and an unpinned on for "tip" environments. Several
dependencies have been missed in the mypy envornments, so this should
make it easier by standardizing environment dependencies to be
consistent across environments.
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
Since typeshed releases are for specific library versions, it makes
more sense to leave them unversioned and allow pip to resolve
dependencies as needed.
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
@holmanb holmanb added the wip Work in progress, do not land label Aug 12, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 12, 2024
@holmanb holmanb force-pushed the holmanb/simplify-tox branch 3 times, most recently from b319403 to 7772ac6 Compare August 13, 2024 03:44
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
When referencing a command from another environment, it will cause
errors when the other environment already exists. Fix it by avoiding
indirection in environment command definitions.

Additionally, simplify envoronment dependency management by defining two
lists of dependencies: a default one with pinned versions for all
environments, and an unpinned on for "tip" environments. Several
dependencies have been missed in the mypy envornments, so this should
make it easier by standardizing environment dependencies to be
consistent across environments.
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
Since typeshed releases are for specific library versions, it makes
more sense to leave them unversioned and allow pip to resolve
dependencies as needed.
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
@holmanb holmanb force-pushed the holmanb/simplify-tox branch 2 times, most recently from 53c7939 to 66979c5 Compare August 13, 2024 03:55
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
When referencing a command from another environment, it will cause
errors when the other environment already exists. Fix it by avoiding
indirection in environment command definitions.

Additionally, simplify envoronment dependency management by defining two
lists of dependencies: a default one with pinned versions for all
environments, and an unpinned on for "tip" environments. Several
dependencies have been missed in the mypy envornments, so this should
make it easier by standardizing environment dependencies to be
consistent across environments.
@holmanb holmanb changed the title Simplify tox, bump test dependencies Simplify tox, bump dependencies Aug 13, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
@holmanb holmanb changed the title Simplify tox, bump dependencies chore: Simplify tox, bump dependencies Aug 13, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
Bump Ubuntu version for better pip dependency resolution.
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

I left some inline comments. I'm assuming that in the black commit, all of the changes there were auto-formatted? Is there anything there that you had to manually update?

pyproject.toml Outdated Show resolved Hide resolved
tox.ini Show resolved Hide resolved
cloudinit/net/__init__.py Show resolved Hide resolved
tox.ini Show resolved Hide resolved
tox.ini Show resolved Hide resolved
.github/workflows/unit.yml Show resolved Hide resolved
@TheRealFalcon TheRealFalcon self-assigned this Aug 13, 2024
@holmanb
Copy link
Member Author

holmanb commented Aug 13, 2024

I left some inline comments. I'm assuming that in the black commit, all of the changes there were auto-formatted?

Correct

Is there anything there that you had to manually update?

This commit was manual changes by me for mypy, but otherwise no.

holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
holmanb added a commit to holmanb/cloud-init that referenced this pull request Aug 13, 2024
@TheRealFalcon
Copy link
Member

Thanks for all the changes! I'm +1 to the PR once the pytest configuration moves back to tox.ini

When referencing a command from another environment, it will cause
errors when the other environment already exists. Fix it by avoiding
indirection in environment command definitions.

Additionally, simplify envoronment dependency management by defining two
lists of dependencies: a default one with pinned versions for all
environments, and an unpinned on for "tip" environments. Several
dependencies have been missed in the mypy envornments, so this should
make it easier by standardizing environment dependencies to be
consistent across environments.
Bump Ubuntu version for better pip dependency resolution.
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

LGTM assuming current checks pass

@holmanb
Copy link
Member Author

holmanb commented Aug 14, 2024

Thanks for all the changes! I'm +1 to the PR once the pytest configuration moves back to tox.ini

The review is much appreciated @TheRealFalcon! I just pushed a change which did that, and also reverted an unnecessary test addition to integration tests and fixed linkcheck which I had broken. I squashed changes before merging, so this is the git diff origin/holmanb/simplify-tox just before I force pushed.

diff --git a/pyproject.toml b/pyproject.toml
index 7eadd316a..2adba3761 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -275,27 +275,3 @@ lint.ignore = [
 
 [tool.ruff.lint.pydocstyle]
 convention = "pep257"
-
-[tool.pytest.ini_options]
-testpaths = "tools tests/unittests"
-addopts = "-vvvv --strict-markers --showlocals --durations 10"
-log_format = "%(asctime)s %(levelname)-9s %(name)s:%(filename)s:%(lineno)d %(message)s"
-log_date_format = "%Y-%m-%d %H:%M:%S"
-markers = [
-    "adhoc: only run on adhoc basis, not in any CI environment (travis or jenkins)",
-    "allow_all_subp: allow all subp usage (disable_subp_usage)",
-    "allow_subp_for: allow subp usage for the given commands (disable_subp_usage)",
-    "ci: run this integration test as part of CI test runs",
-    "ds_sys_cfg: a sys_cfg dict to be used by datasource fixtures",
-    "hypothesis_slow: hypothesis test too slow to run as unit test",
-    "instance_name: the name to be used for the test instance",
-    "integration_cloud_args: args for IntegrationCloud customization",
-    "is_iscsi: whether is an instance has iscsi net cfg or not",
-    "lxd_config_dict: set the config_dict passed on LXD instance creation",
-    "lxd_setup: specify callable to be called between init and start",
-    "lxd_use_exec: `execute` will use `lxc exec` instead of SSH",
-    "serial: tests that do not work in parallel, skipped with py3-fast",
-    "unstable: skip this test because it is flakey",
-    "user_data: the user data to be passed to the test instance",
-    "allow_dns_lookup: disable autochecking for host network configuration",
-]
diff --git a/tox.ini b/tox.ini
index bef415c4f..be5e1d647 100644
--- a/tox.ini
+++ b/tox.ini
@@ -197,7 +197,7 @@ commands = {envpython} -m sphinx -b spelling {posargs:-W doc/rtd doc/rtd_html}
 #
 # followed by manual verification of the links reported
 [testenv:linkcheck]
-deps = {[pinned_versions]deps}
+deps = -r{toxinidir}/doc-requirements.txt
 commands =
     {envpython} -m sphinx {posargs:-b linkcheck doc/rtd doc/rtd_html}
 
@@ -225,9 +225,7 @@ deps = {[latest_versions]deps}
 commands = {envpython} -m isort --check-only --diff {posargs:.}
 
 [testenv:integration-tests]
-deps =
-    {[pinned_versions]deps}
-    -r{toxinidir}/integration-requirements.txt
+deps = -r{toxinidir}/integration-requirements.txt
 commands = {envpython} -m pytest --log-cli-level=INFO {posargs:tests/integration_tests}
 passenv =
     CLOUD_INIT_*
@@ -236,9 +234,7 @@ passenv =
     OS_*
 
 [testenv:integration-tests-ci]
-deps =
-    {[pinned_versions]deps}
-    -r{toxinidir}/integration-requirements.txt
+deps = -r{toxinidir}/integration-requirements.txt
 commands = {envpython} -m pytest --log-cli-level=INFO {posargs:tests/integration_tests}
 passenv =
     CLOUD_INIT_*
@@ -250,9 +246,7 @@ setenv =
 [testenv:integration-tests-jenkins]
 # Pytest's RC=1 means "Tests were collected and run but some of the tests failed".
 # Do not fail in this case, but let Jenkins handle it using the junit report.
-deps =
-    {[pinned_versions]deps}
-    -r{toxinidir}/integration-requirements.txt
+deps = -r{toxinidir}/integration-requirements.txt
 allowlist_externals = sh
 commands = sh -c "{envpython} -m pytest --log-cli-level=INFO {posargs:tests/integration_tests/none} || [ $? -eq 1 ]"
 passenv =
@@ -266,6 +260,30 @@ passenv =
 setenv =
     PYTEST_ADDOPTS="-m not adhoc"
 
+[pytest]
+# TODO: s/--strict/--strict-markers/ once pytest version is high enough
+testpaths = tools tests/unittests
+addopts = --strict
+log_format = %(asctime)s %(levelname)-9s %(name)s:%(filename)s:%(lineno)d %(message)s
+log_date_format = %Y-%m-%d %H:%M:%S
+markers =
+    adhoc: only run on adhoc basis, not in any CI environment (travis or jenkins)
+    allow_all_subp: allow all subp usage (disable_subp_usage)
+    allow_subp_for: allow subp usage for the given commands (disable_subp_usage)
+    ci: run this integration test as part of CI test runs
+    ds_sys_cfg: a sys_cfg dict to be used by datasource fixtures
+    hypothesis_slow: hypothesis test too slow to run as unit test
+    instance_name: the name to be used for the test instance
+    integration_cloud_args: args for IntegrationCloud customization
+    is_iscsi: whether is an instance has iscsi net cfg or not
+    lxd_config_dict: set the config_dict passed on LXD instance creation
+    lxd_setup: specify callable to be called between init and start
+    lxd_use_exec: `execute` will use `lxc exec` instead of SSH
+    serial: tests that do not work in parallel, skipped with py3-fast
+    unstable: skip this test because it is flakey
+    user_data: the user data to be passed to the test instance
+    allow_dns_lookup: disable autochecking for host network configuration
+
 [coverage:paths]
 source =
     cloudinit/

@holmanb holmanb merged commit e1845be into canonical:main Aug 14, 2024
22 checks passed
holmanb added a commit that referenced this pull request Aug 14, 2024
holmanb added a commit that referenced this pull request Aug 14, 2024
When referencing a command from another environment, it will cause
errors when the other environment already exists. Fix it by avoiding
indirection in environment command definitions.

Additionally, simplify envoronment dependency management by defining two
lists of dependencies: a default one with pinned versions for all
environments, and an unpinned on for "tip" environments. Several
dependencies have been missed in the mypy envornments, so this should
make it easier by standardizing environment dependencies to be
consistent across environments.
holmanb added a commit that referenced this pull request Aug 14, 2024
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