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

Test for Windows and MacOS too in addition to Linux #171

Open
niketagrawal opened this issue Nov 14, 2023 · 2 comments
Open

Test for Windows and MacOS too in addition to Linux #171

niketagrawal opened this issue Nov 14, 2023 · 2 comments
Assignees

Comments

@niketagrawal
Copy link
Collaborator

niketagrawal commented Nov 14, 2023

Current behavior

When a pull request is created towards the main branch from a feature branch, tests are triggered automatically by GitHub actions. These tests target the code in the feature branch. Tests are run iteratively for Python 3.9, 3.10, and 3.11 to ensure that the new changes do not break anything and the software's behavior is consistent across the supported Python versions. Tests are currently being run only for Linux environment. These tests are run by GitHub on a remote Linux machine somewhere.

This is specified in the automated tests workflow file .github/workflows/run-tests.yml

runs-on: ubuntu-latest
    timeout-minutes: 15

    strategy:
      matrix:
        python-version: ["3.9", "3.10", "3.11"]

Desired behavior

Test the new changes on Windows and MacOS in addition to testing new changes on Linux. This will help cover the following scenario.

  • Dependencies/libraries needed by aeolis such as numpy, matplotlib, etc., may behave differently on different operating systems or contain operating system (OS) specific bugs. Testing for multiple OS will help catch such errors in advance.

Proposed changes

Specify a matrix of operating systems containing Windows, MacOS, and Linux in .github/workflows/run-tests.yml

strategy:
      matrix:
        os: [ubuntu-latest, windows-latest, macos-latest]
        python-version: ["3.9", "3.10", "3.11"]

    runs-on: ${{ matrix.os }}

Consequences of the proposed changes

  • Currently tests take approximately 3.5 minutes to run. By specifying the tests to also run on Windows and MacOS for 3 Python versions, tests will take approximately 5.5-6 minutes to run based on an experiment in the fork.
  • This extra time taken by the tests to run does not impact the user and developers working with aeolis on their local machines. This extra time is spent only on GitHub's end when a pull request is created. As a reviewer, you will have to wait longer to see if the pull requests passes all the tests.
@niketagrawal niketagrawal self-assigned this Nov 14, 2023
@niketagrawal
Copy link
Collaborator Author

niketagrawal commented Nov 14, 2023

@Sierd, @manuGil do you think this tradeoff between extra time spent to run tests on multiple operating systems (OS) and the added value of catching potential bugs across different OS is reasonable?

@manuGil
Copy link
Collaborator

manuGil commented Nov 15, 2023

@Sierd, @manuGil do you think this tradeoff between extra time spent to run tests on multiple operating systems (OS) and the added value of catching potential bugs across different OS is reasonable?

I think it is, but only for releases. I suggest to create a new action to test installing different OS's. This should be triggered as one of the last steps before making a new release. For example just before the action to push the code to PyPI, and maybe it is a job we want to add to the same workflow file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants