search

gaogaotiantian / objprint

A library that can print Python objects in human readable format
Apache License 2.0
519 stars 43 forks source link

[CI]Pin shell to use as bash #100

Closed cbw512 closed 1 year ago

cbw512 commented 1 year ago

Minor change:As the shell bash is currently supported in GitHub CI windows platform, we can now to not write spaecial code for Windows.

Test for this PR: See the build workflow run in my repo

gaogaotiantian commented 1 year ago

Thank you for noticing this, but the purpose of test is to cover as many scenarios as possible, especially use cases in reality. On Windows, very few people are actually using bash, so we should not move to bash for testing just because it saves us a couple of lines of code in CI file. Especially considering that objprint is a library that "prints" stuff which might be affected by the shell itself. I remember some powershell/CMD specific issues, which would not be caught if bash was used to test.

cbw512 commented 1 year ago

But a color issue will only find out by user even without this change, color is disabled in the tests

Is there any other hidden issue?

gaogaotiantian commented 1 year ago

Color is disabled by default in the tests, but some test will enable it explicitly.

Color is also just an example to explain why we should not use bash on Windows. The problem of hidden issues is that they are hidden - we don't know them. The principle of testing, is to cover as many use cases as possible, and to mimic the environment the user is using as much as possible.

The majority of users on Windows are using Powershell and it should be covered by the tests.

The benefit of testing with bash on Windows is minimal - we made the yml file slightly cleaner - that's it. Okay maybe we also get a better coverage for bash on Windows - which only represents a very small group of users. The cost is significantly larger than the benefit - we lost coverage on the most popular shell on Windows.