Make WordPress Core

Opened 5 months ago

Closed 9 days ago

Last modified 6 days ago

#62325 closed defect (bug) (fixed)

Ensure PHPUnit tests making network requests are in the external-http group.

Reported by: peterwilsoncc's profile peterwilsoncc Owned by: peterwilsoncc's profile peterwilsoncc
Milestone: 6.8 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch has-unit-tests commit
Focuses: Cc:

Description

A number of tests in the PHPUnit test suite make network requests to ensure that features such as oembeds, DNS lookups and remote requests work as intended.

As these tests can be slow and are susceptible to failure in un-ideal network conditions, they ought to be in the external-http group to prevent false negatives when the developers run the test suite locally, especially within a virtual environment.

Some of these tests are missing the group, the purpose of this test is to review the test suite and ensure they are all in the external-http group.

cc @azaozz per discussion on PR#7655.

Change History (11)

#1 @peterwilsoncc
5 months ago

In 59326:

Tests/Build tools: Only fail importer tests if plugin is missing.

Reverts an earlier change to the test suite in which the PHPUnit tests could not run if the importer plugin was not available.

This update allows the test suite to run and will fail importer tests if the plugin is not available.

Follow up to r59085.

Props peterwilsoncc, azaozz.
See #62325.

#2 @peterwilsoncc
5 months ago

In 59327:

Tests/Build tools: Only fail importer tests if plugin is missing.

Reverts an earlier change to the test suite in which the PHPUnit tests could not run if the importer plugin was not available.

This update allows the test suite to run and will fail importer tests if the plugin is not available.

Follow up to r59085.

Merges [59326] to the 6.7 branch.

Props peterwilsoncc, azaozz.
See #62325.

#3 @peterwilsoncc
5 months ago

🤦‍♂️ The above commits has the incorrect ticket number, the were for #61530.

#4 @azaozz
5 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.8

Thanks for opening this ticket!

these tests can be slow and are susceptible to failure in un-ideal network conditions

Yep, thinking that fixing the group will be a nice improvement for most people that run the unit tests locally (the tests can be run with --exclude-group external-http when network connection is slow). Would probably be good to also add a reminder/docs to maintain that group when new tests are added.

Seems this can be done in 6.8. May also be a good-first-bug, perhaps.

This ticket was mentioned in PR #8074 on WordPress/wordpress-develop by @sukhendu2002.


3 months ago
#5

  • Keywords has-patch has-unit-tests added; needs-patch removed

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


13 days ago

@audrasjb commented on PR #8074:


13 days ago
#7

Hey @peterwilsoncc, as per today's scrub, I'm adding you for review ;)

@sukhendu2002 commented on PR #8074:


10 days ago
#8

Hi @peterwilsoncc, Thanks for the feedback! I've made the changes accordingly.

#9 follow-up: @peterwilsoncc
9 days ago

  • Keywords commit added

I've approved the pull request associated with this ticket, I think it's ready for commit.


Yep, thinking that fixing the group will be a nice improvement for most people that run the unit tests locally (the tests can be run with --exclude-group external-http when network connection is slow).

@azaozz By default the group is excluded for local test runs, so once I commit the PR running the local test suite should avoid failures caused by network errors.

#10 @peterwilsoncc
9 days ago

  • Owner set to peterwilsoncc
  • Resolution set to fixed
  • Status changed from new to closed

In 59964:

Build/Test Tools: Update external-http group to include all network tests.

Updates the external-http group in the PHPUnit test suite to include all tests that rely on network requests. This is to ensure the main test suite runs do not contain any tests that can fail due to network conditions.

Props sukhendu2002, azaozz, audrasjb.
Fixes #62325.

#11 in reply to: ↑ 9 @azaozz
6 days ago

Replying to peterwilsoncc:

By default the group is excluded for local test runs

Ah, I see. Sounds good, thanks!

Note: See TracTickets for help on using tickets.