Skip to content

multi enviroment ci workflow#697

Draft
NedislavKolev wants to merge 23 commits into
eclipse-qrisp:mainfrom
NedislavKolev:multiple_enviroments
Draft

multi enviroment ci workflow#697
NedislavKolev wants to merge 23 commits into
eclipse-qrisp:mainfrom
NedislavKolev:multiple_enviroments

Conversation

@NedislavKolev

Copy link
Copy Markdown
Contributor

Description

This workflow allows running the tests in MacOS and Windows environments.

Related Issues

Closes #
Related to #

Type of Change

  • Feature (new functionality)
  • Change Request (modification of existing functionality)
  • Bug Fix
  • Refactoring (no behavior change)
  • Performance improvement
  • Documentation
  • CI / Build

Breaking Change?

  • Yes
  • No

If yes, describe the impact and migration path:

What was changed?

  • Added macos-latest and windows-latest to the Pytest GitHub Actions workflow.
  • Updated pyproject.toml to structure dependency groups cleanly and excluded pyscf on Windows.

How was it tested?

Test-ID Status
T-001 ✅ / ❌
T-002 ✅ / ❌
T-003 ✅ / ❌
T-004 ✅ / ❌

Screenshots / Output (if applicable)

Checklist

  • My code follows the project's coding standards
  • I have performed a self-review
  • I have added/updated tests (referencing issue Test-IDs)
  • All tests pass locally and in CI
  • I have updated the documentation
  • I have added a changelog entry
  • Breaking changes are documented with migration path

Reviewer Notes

Comment on lines +41 to +72
include:
- os: windows-latest
python-version: "3.11.*"
test-group:
name: core
paths: "tests/circuit_tests tests/core_tests tests/primitives_tests tests/jax_tests tests/test_typing.py"
pip-extras: "[dev-windows,xdsl,iqm]"
pytest-flags: '-k "not test_catalyst_interface and not test_qubit_array_fusion"'

- os: windows-latest
python-version: "3.11.*"
test-group:
name: algorithms-and-integrations
paths: "tests/algorithms_tests tests/operators_tests tests/block_encodings_tests tests/interface_tests tests/stim_integration_tests"
pip-extras: "[dev-windows,xdsl,iqm]"
pytest-flags: '--ignore=tests/interface_tests/test_converter_qrisp_to_qml.py'

- os: windows-latest
python-version: "3.12.*"
test-group:
name: core
paths: "tests/circuit_tests tests/core_tests tests/primitives_tests tests/jax_tests tests/test_typing.py"
pip-extras: "[dev-windows,xdsl,iqm]"
pytest-flags: '-k "not test_catalyst_interface and not test_qubit_array_fusion"'

- os: windows-latest
python-version: "3.12.*"
test-group:
name: algorithms-and-integrations
paths: "tests/algorithms_tests tests/operators_tests tests/block_encodings_tests tests/interface_tests tests/stim_integration_tests"
pip-extras: "[dev-windows,xdsl,iqm]"
pytest-flags: '--ignore=tests/interface_tests/test_converter_qrisp_to_qml.py'

@purva-thakre purva-thakre Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

since a lot of these lines are repetitive, you could refactor them into unique lines where you do not have to write the python version by hand. python-version is defined before. The lines you added could directly use that. You also shouldn't add os: windows-latest multiple times. Add it once and then use it in the form of a strategy.

See https://docs.github.com/en/actions/how-tos/write-workflows/choose-what-workflows-do/run-job-variations and https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#using-a-multi-dimensional-matrix

Also, I see certain OS are skipping some unit tests. I assume that module is incompatible with the OS? You should add a comment about each such that someone who edits the workflow in the future knows why the ignore flags were initially added.

@NedislavKolev

Copy link
Copy Markdown
Contributor Author

The refactoring is using a flattened nested object to exactly match the entire object structure. So that Windows can receive its custom overrides. Flat strings match perfectly every time.
Moved pip-extras out of the base matrix because the include block tried to overwrite that array element with ("[dev-windows,xdsl,iqm]") and fell back to the default Linux dependencies.

Removed 'multiple_enviroments' branch from push triggers.
- os: macos-latest
pip-extras: "[dev,catalyst,xdsl,iqm]"
- os: windows-latest
pip-extras: "[dev-windows,xdsl,iqm]"

@purva-thakre purva-thakre Jun 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not too sure if dev-windows is correct here? In the toml file, the group you defined is dev_windows. I am mostly confused why the CI did not raise an error here. maybe I am missing something here 😅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah, this is because of name normalization...will correct it

- All underscores (_) and periods (.) are replaced with hyphens (-).

- All characters are converted to lowercase.

@purva-thakre

Copy link
Copy Markdown
Contributor

Moved pip-extras out of the base matrix because the include block tried to overwrite that array element

Maybe it's just me but I feel the include option is adding unneeded complexity. What if you create steps based on what os the workflow is running on? For example, if runner == windows, then pip install [windows-groups] and same for the flag; you only need to test if it is windows. Thoughts?

@NedislavKolev

Copy link
Copy Markdown
Contributor Author

I had tried this approach previously, but it had problems getting the flags, and it adds complexity to the pip and pytest commands. My opinion is that this is a less structured way of achieving the goal. The include option may be more complex, looking at the config, but it could be better for future development.

Comment thread pyproject.toml
dev-testing-windows = [
"pytest==9.1.0",
"qiskit-aer",
"pyscf",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

simply curious: is pyscf incompatible with windows?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is, alongside catalyst.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good to know! Should we think about documenting this in the readme and the development guide as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

From what I can see, there is no documentation about installing Qrisp in a Windows environment, so I think this is a great suggestion. We can discuss it in the software engineering meeting.

@purva-thakre

purva-thakre commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

I had tried this approach previously, but it had problems getting the flags, and it adds complexity to the pip and pytest commands.

can you help me understand this? how is it making the pip and pytest commands more complex?

We can add a windows specific installation step by avoiding the os redundancy. There is only 1 OS that requires a separate installation group right now.

      - name: Install dependencies
        run: |
          python -m pip install --upgrade pip
          pip install -e "${{ runner.os == 'Windows' && '.[dev_code_style,dev-testing-windows,xdsl,iqm]' || '.[dev,catalyst,xdsl,iqm]' }}"

Then do something similar for the pytest step.

@NedislavKolev

Copy link
Copy Markdown
Contributor Author

Because we are putting the configuration inside the command when we can do it outside. Using include gives us more control and can allow us to add extra options for testing in the future while using GitHub's integrated options.

While this approach could work, it looks more like a workaround. I think we need to do our best to be prepared for the future and follow the best DevOps practises.

      - name: Install dependencies
        run: |
          python -m pip install --upgrade pip
          pip install -e "${{ runner.os == 'Windows' && '.[dev_code_style,dev-testing-windows,xdsl,iqm]' || '.[dev,catalyst,xdsl,iqm]' }}"

Thank you very much for your nice and constructive feedback!

@purva-thakre

purva-thakre commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Sorry if I am dragging this out. Not my intention.

Because we are putting the configuration inside the command when we can do it outside. Using include gives us more control and can allow us to add extra options for testing in the future while using GitHub's integrated options.

Right. My main issue is not that you used include but the redundancy that gets added with the chosen approach. If I look at the how dependencies are installed for the different OSes, the only one that's different is windows. Due to this, I do not understand why you need to specify ubuntu and macos separately. From my POV, for these OSes, the default approach seems straightforward.

          # Assign pip-extras based on OS type
          - os: ubuntu-latest
            pip-extras: "[dev,catalyst,xdsl,iqm]"
          - os: macos-latest
            pip-extras: "[dev,catalyst,xdsl,iqm]"
          - os: windows-latest
            pip-extras: "[dev-windows,xdsl,iqm]"

Since the two steps below follow the same pattern, I wonder if utilizing a matrix makes sense here. Just thinking out loud, not suggesting you to make a change.

          - os: windows-latest
            test-group-name: core
            # Exclude tests that are not supported on Windows due to platform-specific issues
            pytest-flags: '-k "not test_catalyst_interface and not test_qubit_array_fusion"'          
          - os: windows-latest
            # Exclude tests that are not supported on Windows due to platform-specific issues
            test-group-name: algorithms-and-integrations
            pytest-flags: '--ignore=tests/interface_tests/test_converter_qrisp_to_qml.py'

@NedislavKolev

Copy link
Copy Markdown
Contributor Author

That is a really great point, and I completely see what you mean about the visual redundancy!

The reason it’s structured this way comes down to a known limitation in GitHub's matrix engine. If we define a default pip-extras array at the top level, GitHub treats it as a strict matrix dimension. When the include block later tries to override that array element with the Windows-specific string, GitHub gets confused by the type mismatch, silently fails the override, and forces Windows to use the Linux packages anyway.

To make the Windows overrides actually stick, we have to define pip-extras entirely within the include block. Listing Ubuntu and macOS explicitly feels a bit verbose, but it keeps our environment configuration clean and easy to read.

Let me know what you think!

@purva-thakre

purva-thakre commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

If we define a default pip-extras array at the top level, GitHub treats it as a strict matrix dimension. When the include block later tries to override that array element with the Windows-specific string, GitHub gets confused by the type mismatch, silently fails the override, and forces Windows to use the Linux packages anyway.

Do you mind if I take a couple of days reading up on the Github Actions documentation? As far as I remember, when you use pip-extras, include cannot be used as an override which we both agree on. That's a hard rule, not something specific to arrays specifically. I do not recall anything about a type mismatch though.

In the past, I have been able to keep pip-extras out of the top-level matrix entirely, and instead define a single default at the job level via env, overriding it only where needed through a sparse include entry.

Edit: Below is what I have been suggesting.

    env:
      default-pip-extras: "[dev,catalyst,xdsl,iqm]"
    strategy:
      fail-fast: false
      matrix:
        # Actual dimensions that need to be multiplied out
        os: [ubuntu-latest, macos-latest, windows-latest]
        python-version: ["3.11.*", "3.12.*"]
        test-group-name: [core, algorithms-and-integrations]

        include:
          # Map paths to test groups globally
          - test-group-name: core
            test-group-paths: "tests/circuit_tests tests/core_tests tests/primitives_tests tests/jax_tests tests/test_typing.py"
          - test-group-name: algorithms-and-integrations
            test-group-paths: "tests/algorithms_tests tests/operators_tests tests/block_encodings_tests tests/interface_tests tests/stim_integration_tests"

          # Override pip-extras for Windows (others use default)
          - os: windows-latest
            pip-extras: "[dev-windows,xdsl,iqm]"

@NedislavKolev

Copy link
Copy Markdown
Contributor Author

Do you mind if I take a couple of days reading up on the Github Actions documentation?

Not at all, we can continue the discussion until we find the best solution!

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.

2 participants