skipOnAutomatedBranches() does not work as expected

[SergeyBiryukov]

Background: #49049

WP_UnitTestCase_Base::skipOnAutomatedBranches()​ is supposed to make sure some tests only run on trunk/master, not on other branches.

test_readme()​ is one of those tests, however it currently runs (and fails) at least on the 5.3 branch too, see ​job 268747198 for example.

[SergeyBiryukov]

Originally introduced in [40241] / #39486.

Just noted that this only applies to the 5.3 branch. For 5.2 and earlier branches, the tests are skipped as expected, as ​job 266659075 shows.

This appears to be caused by the work on #47767. It looks like TRAVIS_BRANCH and TRAVIS_PULL_REQUEST environment variables are not passed to the Docker container.

Looking closer, the skip condition in ::skipOnAutomatedBranches() could use some further correction. Per ​Travis documentation, if the current job is not a pull request, TRAVIS_PULL_REQUEST contains a string "false", not boolean false, so false !== $travis_pull_request always returns true, whether the job is a pull request or not.

If the goal is to skip on pull requests OR branches, this seems to be the correct condition:

'false' !== $travis_pull_request || 'master' !== $travis_branch

[SergeyBiryukov]

In 46999:

Build/Test Tools: Pass the TRAVIS_BRANCH and TRAVIS_PULL_REQUEST environment variables along to the Docker container.

This ensures that WP_UnitTestCase::skipOnAutomatedBranches() has access to these variables.

See #49050, #47767.

[SergeyBiryukov]

The function_exists( 'getenv' ) check in line 190​ also seems redundant, as we already have two other instances of getenv() without that check in lines 82 and 99​ of includes/bootstrap.php.

WordPress core also uses it without checking in wp_get_update_php_url()​ and wp_get_direct_php_update_url()​.

[SergeyBiryukov]

In 47000:

Tests: Correct the check for pull requests in WP_UnitTestCase_Base::skipOnAutomatedBranches().

Mark the test as failed if the environment variables are unavailable.

Fixes #49050.

[SergeyBiryukov]

[46999], [47000], and [47001] should be backported to the 5.3 branch.

[SergeyBiryukov]

In 47001:

Tests: Don't fail the test in WP_UnitTestCase_Base::skipOnAutomatedBranches() if Travis environment variables are unavailable, it prevents from running the test locally.

Follow-up to [47000].

See #49050.

[SergeyBiryukov]

In 47125:

Build/Test Tools: Pass the TRAVIS_BRANCH and TRAVIS_PULL_REQUEST environment variables along to the Docker container.

This ensures that WP_UnitTestCase::skipOnAutomatedBranches() has access to these variables.

Correct the check for pull requests in WP_UnitTestCase_Base::skipOnAutomatedBranches().

Merges [46999], [47000], and [47001] to the 5.3 branch.
Fixes #49050.

[desrosj]

In 49267:

Build/Test Tools: Pass GitHub Action related environment variables to the Docker container.

This ensures that WP_UnitTestCase::skipOnAutomatedBranches() has access to these variables so that time sensitive tests can be skipped when appropriate.

This also updates that logic to be more clear.

Follow up to [49264].

Props ocean90, johnbillion.
See #50401, #49050, #47767.