Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#41345 closed enhancement (fixed)

'import' group tests fail using git repository

Reported by: enricosorcinelli's profile enrico.sorcinelli Owned by: jnylen0's profile jnylen0
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.9
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:

Description

Recently I switched to git for contributing to WordPress core. And of course, for convenience, I run tests under git tree.

In this situation, all tests pass with the exception of import group tests, since they require WordPress Import plugin that doesn't live under git working tree (these changesets [40532] and [40531] forced import group tests to fail instead of skip and they seems to revert the [27349]).

In the svn tree this doesn't occur because of svn:externals set to wordpress-importer.

As workaround we could alternatively:

  • launch phpunit tests with --exclude-group import option or we
svn co https://plugins.svn.wordpress.org/wordpress-importer/trunk/ wordpress-importer
  • under tests/phpunit/data/plugins/ directory and svn up it at regular basis, even if I think that the proposed patch would be a cleanier approach.

Anyway, in https://make.wordpress.org/core/handbook/contribute/git/, a notes about specific test actions above should be added.

PS: Probably there is, but I did not find a git repository for WordPress Importer plugin in order to add as submodule.

Attachments (3)

41345.patch (789 bytes) - added by enrico.sorcinelli 6 years ago.
41345.2.patch (1.5 KB) - added by enrico.sorcinelli 6 years ago.
Improved skipped test message
41345.3.patch (2.2 KB) - added by enrico.sorcinelli 6 years ago.

Download all attachments as: .zip

Change History (11)

#1 @enrico.sorcinelli
6 years ago

  • Keywords has-patch added

This ticket was mentioned in Slack in #core-php by enrico.sorcinelli. View the logs.


6 years ago

@enrico.sorcinelli
6 years ago

Improved skipped test message

#3 @jnylen0
6 years ago

  • Keywords needs-refresh added
  • Owner set to jnylen0
  • Status changed from new to assigned

I don't think we should skip these tests - the changes in [40531] and [40532] were intentional. Without those changes, if someone has an incomplete test environment and they write patches against plugins or another feature that is left out of their local testing, then they're increasing the chance of writing a bad patch that fails a build later on. Quoting from #40533:

If the test data isn't valid or the environment is not capable of supporting the test then the test should fail, otherwise the tests are not reliable.

I added a new section to the handbook page on git that explains what is going on: https://make.wordpress.org/core/handbook/contribute/git/#unit-tests

Let's leave the test failures in place, but link to this new documentation from the test failure messages. Something like this:

This test requires the WordPress Importer plugin to be installed. See: https://make.wordpress.org/core/handbook/contribute/git/#unit-tests

#4 @enrico.sorcinelli
6 years ago

Now you've updated the handbook by explaining how to fix the failure when testing with git, I updated the patch by modifying the group import tests failure messages as you suggested.

#5 @jnylen0
6 years ago

  • Keywords needs-refresh removed
  • Milestone changed from Awaiting Review to 4.9
  • Status changed from assigned to accepted

Thanks for the patch. This will undoubtedly help people who are getting started with running the unit tests from a git repo.

#6 @jnylen0
6 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 41090:

Tests: Be more helpful when the wordpress-importer plugin is missing.

Link to some documentation that explains the problem and how to resolve it.

Props enrico.sorcinelli.
Fixes #41345.

#7 @johnbillion
6 years ago

In 41169:

Build/Test Tools: Clarify the error message when running the test suite without the WordPress Importer plugin present in the test suite.

See #41345

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


4 years ago

Note: See TracTickets for help on using tickets.