Skip to content

Don't test without opcache on push #11867

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

Closed
wants to merge 1 commit into from

Conversation

iluuu1994
Copy link
Member

@iluuu1994 iluuu1994 commented Aug 3, 2023

It's very rare for something to fail without opcache but not with opcache. We will still run without opcache in nightly. One valuable information lost here is whether something is caused by opcache/JIT. But this can be verified locally when assessing the problem. Alternatively we may look into triggering specific jobs on-demand.

Let me know what you think. I don't want to remove something useful, but it seems like it wouldn't make much of a difference.

It's very rare for something to fail with opcache but not without opcache. One
valuable information lost here is whether something is cause by opcache/JIT. But
this can be verified locally when assessing the problem. Alternatively we may
look into triggering specific jobs on-demand.
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

This wouldn't have caught the failures on #11850 as it works with opcache but not without it. So maybe only keep it on the X86-64 debug build?

@TimWolla
Copy link
Member

TimWolla commented Aug 3, 2023

Not unnecessarily heating the planet is a good thing and this should also reduce the turn-around time for test results to arrive which is also a good thing. I also agree with Girgias' suggestion to keep it for one of the jobs, because “both-OS-and-non-opcache-specific” failures should likely be super rare.

@iluuu1994 iluuu1994 closed this in 5cd0208 Aug 7, 2023
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.

3 participants