Don't use return if ENV["CI"] in formulae

[MikeMcQuaid]

These broadly fit into two categories:

• skipping test do blocks
• skipping post_install operations

These are a bad idea for multiple reasons:

• We're skipping entire integration tests blocks but have a brew audit that isn't flagging up that these formulae are untested. Additionally, we're downloading these bottles and running brew test on them when we test dependents which does nothing.
• test do exists for our integration testing. It's not useful to have these blocks do nothing in CI where they are relied upon
• skipping post_install with ENV["CI"] means if someone uses this formula to install a package in GitHub Actions: it's not going to work for them as documented/expected

[carlocab]

We'd need an alternative solution to avoid post-install failures on CI when we simultaneously test the different mariadb and related formulae.

Context:

#66450 (comment) (and the next four or so comments)
#66727

[jonchang]

It would be nice to not need these, but they were all added for a good reason and shouldn't be removed without carefully figuring out a better solution.

Prior to #52004 being merged we regularly had to reimage and redeploy VMs because of confusing and hard-to-diagnose issues around postinstall blocks. This will be an even bigger problem for the ARM machines as those run on bare metal and can't be torn down and set up in the same way as the Intel VMs.

I would prefer ENV["CI"] in homebrew/core if the alternative is to watch the actions queue and regularly rebuild VMs.

[dbussink]

I would prefer ENV["CI"] in homebrew/core if the alternative is to watch the actions queue and regularly rebuild VMs.

The problem we hit was that we're using Actions to test a library on MacOS. That library requires MySQL and we want to make sure it works for people on their local development machine. Actions sets CI in general, so Homebrew thought it would be running for its own CI, and the data directory never was created. This resulted in the CI tests failing with no clear description as to why. Homebrew claimed even that brew services start mysql worked in the job, even though it failed.

The issue seems to be that the assumption of CI is set, we're running the Homebrew CI setup is wrong. It can very well be any other Actions job of someone using Homebrew to install some package for their own testing. If Homebrew has a need to know if it's running its own CI suite, maybe a separate environment set in the Homebrew CI job is better? So like HOMEBREW_CI or whatever name would be more appropriate?

[MikeMcQuaid]

The issue seems to be that the assumption of CI is set, we're running the Homebrew CI setup is wrong. It can very well be any other Actions job of someone using Homebrew to install some package for their own testing. If Homebrew has a need to know if it's running its own CI suite, maybe a separate environment set in the Homebrew CI job is better? So like HOMEBREW_CI or whatever name would be more appropriate?

Yes, I agree with @dbussink, this is the the issue here. What I'd propose is we:

• check for HOMEBREW_GITHUB_ACTIONS or GITHUB_ACTIONS_HOMEBREW_SELF_HOSTED or similar instead of CI in postinstall do blocks
• delete everything after return if ENV["CI"] in test do blocks
• if this means the test do blocks are empty: remove the test do blocks rather than have the (false) confidence that we're actually testing something when we're not

[gromgit]

• delete everything after return if ENV["CI"] in test do blocks

A quick check seems to show that all the test blocks so affected involve a PostgreSQL database. It may instead be best to review these cases, to see if they can actually be run productively under CI.

Does anyone remember why these tests were disabled?

[MikeMcQuaid]

Does anyone remember why these tests were disabled?

Issues with shared memory filling up on our CI workers and no way we could find to configure postgresql to not use shared memory in this way (or to clear it). The best fix for now seems to be replacing these with e.g. --version tests (as currently we're testing nothing).