Make WordPress Core

Opened 5 months ago

Closed 3 months ago

#25913 closed defect (bug) (worksforme)

Investigate why we skip so many Unit Tests

Reported by: wonderboymusic Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.8
Component: Unit Tests Keywords:
Focuses: Cc:


My hunch as that we shouldn't be skipping so many tests. I am creating this ticket as a stake to point commits and comments (if necessary).

Change History (11)

comment:1 nacin5 months ago

Nearly all skipped tests are because associated tickets are still open.

comment:2 follow-up: wonderboymusic5 months ago

After running phpunit --verbose: a majority, yes. Also a bunch skip when there is no ImageMagick.

comment:3 in reply to: ↑ 2 bpetty5 months ago

Replying to wonderboymusic:

Also a bunch skip when there is no ImageMagick.

Yep, all pretty normal. What would you expect it to do?

We also conditionally include PHP 5.3+ tests, though these are filtered out even earlier so they aren't even included in the skipped test numbers.

comment:4 wonderboymusic5 months ago

Since these tickets all have tests, would be good to see why they are still open

comment:5 follow-up: wonderboymusic5 months ago

In 26093:

The test for #5953 doesn't require the ticket to be closed for its assertions to pass.

See #25913.

comment:6 in reply to: ↑ 5 SergeyBiryukov5 months ago

Replying to wonderboymusic:

The test for #5953 doesn't require the ticket to be closed for its assertions to pass.

There was a consensus some time ago to keep @ticket references even after the ticket is closed (so that we could quickly find the ticket if the test fails in the future):

There was a performance issue caused by querying Trac for each ticket separately, but that was fixed in [891/tests].

It's possible to forcibly run skipped tests either by specifying the ticket as a group (phpunit --group 5953), or by uncommenting WP_TESTS_FORCE_KNOWN_BUGS (for all tickets): tags/3.7.1/wp-tests-config-sample.php#L10.

I don't see a reason to remove @ticket references.

comment:7 wonderboymusic5 months ago

nacin shared similar sentiments via Skype - that particular unit test doesn't actually call the code it planned on testing, and the ticket remains open due to concerns around realpath()

comment:8 wonderboymusic5 months ago

The gist: Canonical - there are a bunch of tests skipped for canonical redirect tickets that aren't closed yet. When I get the courage/energy, gonna look at those

comment:9 nacin5 months ago

In 26552:

Restore @ticket reference. Reverts [26093], see #25913.

comment:10 nacin5 months ago

  • Milestone changed from 3.8 to Future Release

comment:11 nacin3 months ago

  • Milestone Future Release deleted
  • Resolution set to worksforme
  • Status changed from new to closed

Seems like this can be closed. For @ticket references that work without them — let's go to the tickets themselves and see what we can do to adjust things there.

Note: See TracTickets for help on using tickets.