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

ci: Skip package build on tox runs #5210

Merged
merged 1 commit into from
Apr 26, 2024
Merged

Conversation

TheRealFalcon
Copy link
Member

@TheRealFalcon TheRealFalcon commented Apr 25, 2024

Proposed Commit Message

ci: Skip package build on tox runs

Building a wheel/sdist generally adds 5+ seconds to every tox run.
This is unnecessary because a built package isn't needed to run any
of the CI tasks.

Also remove the `recreate` line as it wasn't doing anything. To work
correctly, it should be defined under `[testenv]`,
not `[tox]`.

Additional Context

tox -e ruff is near instant on Python 3.8+

recreate lives under https://tox.wiki/en/latest/config.html#tox-environment

Test Steps

Checklist

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>)

@TheRealFalcon TheRealFalcon added the wip Work in progress, do not land label Apr 25, 2024
Building a wheel/sdist generally adds 5+ seconds to every tox run.
This is unnecessary because a built package isn't needed to run any
of the CI tasks.

Also remove the `recreate` line as it wasn't doing anything. To work
correctly, it should be defined under `[testenv]`,
not `[tox]`.
@TheRealFalcon TheRealFalcon removed the wip Work in progress, do not land label Apr 25, 2024
Copy link
Member

@holmanb holmanb left a comment

Choose a reason for hiding this comment

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

I like this idea. We won't exercise package build as much as a result, but I don't see that as a loss. We do that in the CI integration job, and plenty in our integration tests too.

Maybe give other other devs a chance to object before merging?

Copy link
Contributor

@aciba90 aciba90 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

LGTM. Let's try this out to ease the time costs of CI and local tox runs to cut down on the recurring cost of development and testing. While I like that our CI tests happen to exercise the package build, We can easily run any of the following on PRs that happen to be packaging related

  • make deb
  • ./packages/bbdeb -S
  • or tox --override testenv.package=wheel or something to validate no degradation in behavior on those branches

@TheRealFalcon TheRealFalcon merged commit acc68de into canonical:main Apr 26, 2024
28 checks passed
@TheRealFalcon TheRealFalcon deleted the toxic branch April 26, 2024 20:25
holmanb added a commit to holmanb/cloud-init that referenced this pull request May 1, 2024
Commit acc68de introduced a change which no longer builds a wheel,
however integration tests now fail when dependencies are not available.

Include the base requirements in test-requirements.txt.

Fixes canonicalGH-5210
holmanb added a commit that referenced this pull request May 1, 2024
Commit acc68de introduced a change which no longer builds a wheel,
however integration tests now fail when dependencies are not available.

Include the base requirements in test-requirements.txt.

Fixes GH-5210
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.

None yet

4 participants