Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#25913 closed defect (bug) (worksforme)

Investigate why we skip so many Unit Tests

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

Description

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)

#1 @nacin
11 years ago

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

#2 follow-up: @wonderboymusic
11 years ago

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

#3 in reply to: ↑ 2 @bpetty
11 years 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.

#4 @wonderboymusic
11 years ago

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

#5 follow-up: @wonderboymusic
11 years ago

In 26093:

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

See #25913.

#6 in reply to: ↑ 5 @SergeyBiryukov
11 years 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):
https://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2011-11-02&sort=asc#m327088
https://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2012-07-08&sort=asc#m417979

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.

#7 @wonderboymusic
11 years 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()

#8 @wonderboymusic
11 years 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

#9 @nacin
11 years ago

In 26552:

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

#10 @nacin
11 years ago

  • Milestone changed from 3.8 to Future Release

#11 @nacin
11 years 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.