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

[ci] introduce mode: experimental flag #335

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

connorhu
Copy link
Collaborator

@connorhu connorhu commented Mar 13, 2024

@thePanz This was the best solution I found. If mode: experimental is enabled, the exit status is printed but exits the shell with a zero.

This changes is because of php8.4 testing. We can now mark php version as experimental.

@connorhu connorhu force-pushed the feature/gha-ci-experimantal-flag branch from 9032740 to 456ee6d Compare March 13, 2024 08:50
@connorhu connorhu changed the title [ci] introduce mode: experimental flag [ci] introduce mode: experimental flag Mar 13, 2024
@connorhu connorhu requested a review from thePanz March 14, 2024 06:11
@connorhu connorhu force-pushed the feature/gha-ci-experimantal-flag branch 2 times, most recently from 6aa83bb to e7fcce1 Compare March 18, 2024 10:04
@connorhu connorhu marked this pull request as ready for review March 18, 2024 10:04
@connorhu connorhu requested a review from thirsch March 18, 2024 10:06
@connorhu
Copy link
Collaborator Author

If this works, we can roll out the solution to other repos and start testing it with php 8.4.

connorhu added a commit to connorhu/symfony1 that referenced this pull request Mar 18, 2024
@@ -59,4 +59,14 @@ jobs:
run: php data/bin/check_configuration.php

- name: Run Tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would continue-on-error work here as well? Maybe with something like

- name: Run Tests
  run: ...
  continue-on-error: ${{ matrix.mode == "experimental"}}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is possible that it works. Why didn't I find it in the documentation when I searched?

Copy link
Contributor

@mentalstring mentalstring Mar 19, 2024

Choose a reason for hiding this comment

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

I believe the continue-on-error will only make sure the other PHP versions still run and not abort as soon as one fails. But if one fails, the CI will still be marked as failed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's true! The status is also important here to keep the result green.

@mentalstring
Copy link
Contributor

mentalstring commented Mar 19, 2024

IMHO, I'm not sure this hack is worth the time and added complexity, given that a) php8.4 is at least 8 months away from .0 release (why test something that may still change?) and b) our tests are already passing with php8.4 anyway.
We could just let #334 test php8.4 (or wait until rc1 before merging it), and iff it starts failing, at that point either fix our code to make it pass, or temporarily stop testing php8.4 until we have a fix.

@connorhu
Copy link
Collaborator Author

I think in our case (seeing our reaction time) it is very important to foresee what the new version will bring (and run tests with that version). I'm not sure if it's worth it to make it switchable either, but in the end it's not a big change (I'd rather test it with phpunit).

@connorhu connorhu force-pushed the feature/gha-ci-experimantal-flag branch from e7fcce1 to 5b0e526 Compare March 19, 2024 11:24
@alquerci
Copy link
Contributor

Like a feature flag

The overall test suite should pass when the feature flag is enabled or disabled.

Regarding this principle, a failed test should always fail the job.

But how to progressively make symfony1 compatible with next PHP version?

Test case after test case.

  1. Starts with one test file, passes on next PHP version.
  2. Ends with the overall test suite, passes on next PHP version.

With this plan, each contributor can provide a small PR with one test file executed on the next PHP version.

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.

4 participants