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

Run unit tests across Python versions 3.10-3.12. #326

Merged
merged 13 commits into from
Dec 11, 2024

Conversation

ScottTodd
Copy link
Member

@ScottTodd ScottTodd commented Dec 10, 2024

Also updated enough of the project to pass these new test configurations:

@ScottTodd ScottTodd force-pushed the ci-matrix branch 4 times, most recently from 31690ab to 74194fd Compare December 10, 2024 18:54
@ScottTodd ScottTodd changed the title Run unit tests across Python versions 3.10-3.13. Run unit tests across Python versions 3.10-3.12. Dec 10, 2024
@ScottTodd ScottTodd marked this pull request as ready for review December 10, 2024 20:03
@ScottTodd ScottTodd requested a review from Hardcode84 December 10, 2024 20:06
name: "Unit Tests and Type Checking"
name: "${{ matrix.os }} :: ${{ matrix.version }} :: Unit Tests and Type Checking"
Copy link
Member Author

Choose a reason for hiding this comment

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

@marbre you may have opinions on the job naming. I'm not sure about this.

BTW, type checking does make sense to run on multiple python versions. The use of logging.getLevelNamesMapping() # Added in 3.11 was caught by running mypy on 3.10. I originally wanted to move type checking to pre-commit since it's similar to other lint checks, but I now think it makes sense to keep it here

Copy link
Member

Choose a reason for hiding this comment

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

Following the naming in other repos we would have "Unit Tests and Type Checking :: ${{ matrix.os }} :: ${{ matrix.version }}" but to be honest I don't really care and if we start to dislike we can change it in the future. Thus I don't want to be picky here 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I'd like to put important information at the start of the name so it doesn't get cut off

  • image
  • image

Copy link
Member

Choose a reason for hiding this comment

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

Works for me :)

Copy link
Member

@marbre marbre left a comment

Choose a reason for hiding this comment

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

Looks good and is a nice improvement!

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
name: "Unit Tests and Type Checking"
name: "${{ matrix.os }} :: ${{ matrix.version }} :: Unit Tests and Type Checking"
Copy link
Member

Choose a reason for hiding this comment

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

Following the naming in other repos we would have "Unit Tests and Type Checking :: ${{ matrix.os }} :: ${{ matrix.version }}" but to be honest I don't really care and if we start to dislike we can change it in the future. Thus I don't want to be picky here 😉

Co-authored-by: Marius Brehler <marius.brehler@gmail.com>
name: "Unit Tests and Type Checking"
name: "${{ matrix.os }} :: ${{ matrix.version }} :: Unit Tests and Type Checking"
Copy link
Member Author

Choose a reason for hiding this comment

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

Argh GitHub. This was also being used for a required check matching Unit Tests and Type Checking (3.11, ubuntu-22.04), but that check is actually picking up TK CI / Unit Tests and Type Checking (3.11, ubuntu-22.04) also.

I might just copy this snippet from IREE to have a "ci-summary" job with a unique name we can set as required: https://github.com/iree-org/iree/blob/7177c29f9b2d9e255b63987f5dfff174ec2afc2f/.github/workflows/ci.yml#L201-L236

@ScottTodd ScottTodd merged commit bc35630 into iree-org:main Dec 11, 2024
10 checks passed
@ScottTodd ScottTodd deleted the ci-matrix branch December 11, 2024 00:25
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