Skip to content
This repository was archived by the owner on Jul 16, 2025. It is now read-only.

Conversation

@Swatinem
Copy link
Contributor

@Swatinem Swatinem commented Mar 25, 2025

This adds Windows and macOS to our CI test matrix.

It also changes quite a lot of internal functionality and tests to use Path.as_posix() which always prints paths with forward slashes.
This was primarily done to make a ton of tests work on Windows, but IMO it is also a good idea to normalize paths internally to use forward slashes.


As #661 is primarily an issue on Windows, it would be nice to run tests on Windows as well.
And also macOS as well, why not. This should cover the major OSs that folks will be running in their own CI jobs that are using the CLI.

@Swatinem Swatinem self-assigned this Mar 25, 2025
@github-actions
Copy link

github-actions bot commented Mar 25, 2025

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

@codecov
Copy link

codecov bot commented Mar 25, 2025

❌ 5 Tests Failed:

Tests completed Failed Passed Skipped
3585 5 3580 0
View the top 3 failed test(s) by shortest run time
api.temp.calculator.test_calculator::test_divide
Stack Traces | 0.001s run time
def
                test_divide():
                > assert Calculator.divide(1, 2) == 0.5
                E assert 1.0 == 0.5
                E + where 1.0 = <function Calculator.divide at 0x104c9eb90>(1, 2)
                E + where <function Calculator.divide at 0x104c9eb90> = Calculator.divide
                .../temp/calculator/test_calculator.py:30: AssertionError
api.temp.calculator.test_calculator::test_divide
Stack Traces | 0.001s run time
def
                test_divide():
                > assert Calculator.divide(1, 2) == 0.5
                E assert 1.0 == 0.5
                E + where 1.0 = <function Calculator.divide at 0x104c9eb90>(1, 2)
                E + where <function Calculator.divide at 0x104c9eb90> = Calculator.divide
                .../temp/calculator/test_calculator.py:30: AssertionError
api.temp.calculator.test_calculator::test_divide
Stack Traces | 0.001s run time
def
                test_divide():
                > assert Calculator.divide(1, 2) == 0.5
                E assert 1.0 == 0.5
                E + where 1.0 = <function Calculator.divide at 0x104c9eb90>(1, 2)
                E + where <function Calculator.divide at 0x104c9eb90> = Calculator.divide
                .../temp/calculator/test_calculator.py:30: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

This adds Windows and MacOS to our CI test matrix.
@Swatinem Swatinem force-pushed the swatinem/test-win branch from d7318cd to 374cbac Compare March 25, 2025 12:24
@Swatinem Swatinem force-pushed the swatinem/test-win branch from 412d612 to d710977 Compare March 25, 2025 12:48
Copy link
Contributor

@joseph-sentry joseph-sentry left a comment

Choose a reason for hiding this comment

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

i think for the changes in folder_searcher to work with file finding you would need to make sure that the multipart_include_regex is also using as_posix(), this happens here:

multipart_include_regex = globs_to_regex(

@Swatinem Swatinem requested a review from joseph-sentry March 26, 2025 16:51
Copy link
Contributor

@joseph-sentry joseph-sentry left a comment

Choose a reason for hiding this comment

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

i think there's some required checks settings in the repo which is causing the CI to expect certain checks with diff names @thomasrockhu-codecov do you have perms to change those?

@thomasrockhu-codecov
Copy link
Contributor

@joseph-sentry done

@Swatinem Swatinem merged commit 2165dc1 into main Mar 27, 2025
21 checks passed
@Swatinem Swatinem deleted the swatinem/test-win branch March 27, 2025 07:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants