Skip to content

Comments

Added test workflow to install builded wheels before the upload#58

Open
jakub-kocka wants to merge 6 commits intomainfrom
feat/added_tests
Open

Added test workflow to install builded wheels before the upload#58
jakub-kocka wants to merge 6 commits intomainfrom
feat/added_tests

Conversation

@jakub-kocka
Copy link
Collaborator

@jakub-kocka jakub-kocka commented Jan 26, 2026

Description

  • Added/Fixed unit tests and made them automatically run on PR

  • Added installation tests for built wheels before upload to Espressif's PyPI

    • Also, another layer of exclusion mechanism is added for the dependencies of dependencies - everything that we have actually built is again verified with the exclude_list and removed from the packages which will be uploaded
    • Merged wheels in the repair step of the workflow to actually test platform-independent wheels on all platforms and Python versions in the test step
  • Introduced architecture resolver for exclude_list.yaml

  • Added another verification workflow that runs on every PR to actually check what is uploaded in the S3 and if there are exclude list "violations" - might be from manual wheels upload (highly likely not complete wheel) ... this check also runs after the upload of main builds to check if we have not uploaded anything that we don't want to

  • Fixed the new issues - https://github.com/espressif/idf-python-wheels/actions/runs/21887651772

  • Upload has been enhanced in logging scope as well to actually show a completely new package being uploaded (green colour and ++), also statistics are implemented at the end of uploading:

---------- STATISTICS ---------- 
New wheels: 140 
Existing wheels (re-uploaded): 1298
Total uploaded: 1438
---------- END STATISTICS ---------- 

Related

  • internal tracker: IDF-14066

Testing

Unit tests: https://github.com/espressif/idf-python-wheels/actions/runs/22177100784?pr=58
Action run platforms-dispatch: https://github.com/espressif/idf-python-wheels/actions/runs/22177171603
Action run defined-wheel: https://github.com/espressif/idf-python-wheels/actions/runs/22177157441


Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • Documentation is updated as needed.
  • Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

@jakub-kocka jakub-kocka self-assigned this Jan 26, 2026
@github-actions
Copy link

github-actions bot commented Jan 26, 2026

Warnings
⚠️ Please consider squashing your 6 commits (simplifying branch history).
Messages
📖 This PR seems to be quite large (total lines of code: 1843), you might consider splitting it into smaller PRs

👋 Hello jakub-kocka, we appreciate your contribution to this project!


Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- Addressing info messages (📖) is strongly recommended; they're less critical but valuable.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against 483d363

@jakub-kocka jakub-kocka force-pushed the feat/added_tests branch 3 times, most recently from d660b51 to 3994296 Compare January 29, 2026 08:27
@jakub-kocka jakub-kocka marked this pull request as draft January 29, 2026 13:22
@jakub-kocka jakub-kocka force-pushed the feat/added_tests branch 21 times, most recently from d3e68a3 to f92957f Compare February 2, 2026 08:25
@jakub-kocka jakub-kocka force-pushed the feat/added_tests branch 2 times, most recently from 988982b to 2687841 Compare February 9, 2026 07:49
@jakub-kocka jakub-kocka force-pushed the feat/added_tests branch 5 times, most recently from 7718581 to 502fe66 Compare February 12, 2026 12:11
@jakub-kocka jakub-kocka force-pushed the feat/added_tests branch 2 times, most recently from cbbc0dc to 92249a1 Compare February 17, 2026 12:17
@jakub-kocka jakub-kocka requested a review from Copilot February 17, 2026 12:53
@jakub-kocka jakub-kocka marked this pull request as ready for review February 17, 2026 12:59
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR strengthens the wheel build/release pipeline by adding CI workflows and helper scripts to (1) test-install repaired wheels before upload and (2) validate uploaded wheels against exclude_list.yaml, while also making exclude-list processing platform-aware.

Changes:

  • Added new CI workflows for unit tests, cross-platform wheel installation testing, and post-upload S3 verification against exclude_list.yaml.
  • Introduced new tooling/scripts to extract excluded packages, test wheel installation, and verify S3 contents; updated exclude-list and platform resolution logic.
  • Enhanced S3 upload logging/statistics and made repair step tolerant of “no wheels to repair”.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
yaml_list_adapter.py Adds platform filtering and platform-marker normalization for exclude/include YAML processing.
verify_s3_wheels.py New script to scan S3 wheels and detect exclude-list violations.
upload_wheels.py Adds “new vs existing” upload logging and end-of-run statistics.
test_wheels_install.py New CI script to filter/install wheels locally and remove incompatible/excluded wheels.
test_build_wheels.py Reworks/adds unit tests for YAML adapter and helper functions.
repair_wheels.py Treats “no wheels found” as a successful no-op.
os_dependencies/ubuntu.sh Expands PyGObject-related deps (incl. runtime libs) and improves install formatting.
os_dependencies/manylinux.sh Adds more introspection/cairo deps and best-effort girepository installs for repair.
os_dependencies/linux_arm.sh Makes sudo optional and expands/softens ARM dependency installs for CI containers.
extract_exclude_packages.py New utility to list excluded packages for a given platform/Python version.
exclude_list.yaml Updates platform taxonomy and refines/extends exclusion rules and comments.
build_wheels_from_file.py Adds --ci-tests mode to expect failures when building excluded packages.
build_wheels.py Loads exclude list filtered to the current platform.
_helper_functions.py Adds platform resolver, wheel parsing, and exclude-check helpers for install/S3 verification.
.github/workflows/wheels-repair.yml Skips repair steps when no wheels and adds a “merge all repaired wheels” job.
.github/workflows/upload-python-wheels.yml Switches upload input to “tested wheels” artifacts and merges them.
.github/workflows/unit-tests.yml New workflow for unit tests + exclude-build checks + S3 verification on PRs.
.github/workflows/test-wheels-install.yml New reusable workflow to test-install repaired wheels across OS/Python matrices.
.github/workflows/build-wheels-python-dependent.yml Adds Windows OS dependency installation step.
.github/workflows/build-wheels-platforms.yml Inserts test-install gate before upload and adds post-upload S3 verification.
.github/workflows/build-wheels-defined.yml Adds platform selector, ARMv7 legacy build option, and integrates test-install + S3 verify.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jakub-kocka
Copy link
Collaborator Author

There are some "violations" of the exclude_list in the actually uploaded wheels on S3.
Not sure if we want to remove all of them or introduce some skip mechanism.
https://github.com/espressif/idf-python-wheels/actions/runs/22177100784/job/64128503011?pr=58

@dobairoland
Copy link
Collaborator

@jakub-kocka There must be something wrong because on the list of violations there are two esptool packages and I removed all of them after the last incident.

I can delete them but please check why are they are so we won't delete anything by mistake. And please send me the list of full patch as they should be specified at the time of running the command.

required: false
default: true
os_linux_armv7_legacy:
description: Build on linux armv7 legacy (bullseye, glibc 2.31)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe state why do we need "legacy" build. Then it will be clear in the future when such legacy build can be removed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why isn’t this documented anywhere? It appears to have been introduced in #56, but there’s no mention of the change or its rationale. This really should be described in much more detail, especially if it’s intended as a workaround for patchelf.

run: python -m pip install -r build_requirements.txt

- name: Install additional OS dependencies - Windows
run: powershell -ExecutionPolicy Bypass -File os_dependencies/windows.ps1
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to check platform for OS Windows?

-w /work \
-e GH_TOKEN="${GH_TOKEN}" \
-e PIP_NO_CACHE_DIR=1 \
-e LDFLAGS="-Wl,-z,max-page-size=0x1000" \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why such max-page-size?

@jakub-kocka
Copy link
Collaborator Author

@jakub-kocka There must be something wrong because on the list of violations there are two esptool packages and I removed all of them after the last incident.

I can delete them but please check why are they are so we won't delete anything by mistake. And please send me the list of full patch as they should be specified at the time of running the command.

Yes, there was, indeed.
But this is fixed by this PR, the packages got uploaded cca a week ago by the scheduled pipeline. Fortunatelly they should not be broken as it was in the defined wheels action.

I will investigate this and let you know, thanks.

@dobairoland
Copy link
Collaborator

@jakub-kocka LGTM in general but I haven't checked every line.

The main problem with this repository that there is too much done in Gitlab YAML config files. That makes the review hard (i.e. noticing potential issues). The checking is done at several places. It can happen that we add another upload and forgot about implementing the check. This was the issue before as well: the upload happened in so many different cases that we missed something. Please think about it if there could be one script (shell, Python) which would do the upload and would ensure that the exclude list is always honored. Then it wouldn't be possible to bypass the checks.

Copy link
Collaborator

@fhrbata fhrbata left a comment

Choose a reason for hiding this comment

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

FWIW LGTM, thank you!

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.

3 participants