1. The Testing profile can be used already.

    #1366232: drupalCreateUser() breaks if testing site/profile does not install Comment module was one of the main blockers.

  2. It significantly speeds up test runs.
  3. It should be used by default.
  4. Some/many tests may still need to use Standard profile. They can.

    We are not blocked on #913086: Allow modules to provide default configuration for running tests - obviously, we want all tests to run isolated and with explicitly enabled functionality, but until that is possible, it's perfectly fine to use the Standard profile. We can fix and convert those later.

  5. Other Testing system issues I'm working on will speed up the Testing profile even further.

Dependencies

Functional bugs:

  1. #1375134: Random test failure in TaxonomyTermTestCase::testNodeTermCreationAndDeletion()
  2. #1375452: Renaming a content type bundle causes notices on manage fields page (test improvements)
  3. #1373360: Fatal error if optional Block module is not enabled
  4. #1376164: Format SimpleTestFunctionalTest description according to standards
  5. #1376166: Custom logo and favicon functionality inanely tries to support absolute local file paths
  6. #1213536: Non-resettable theme_get_registry() cache causes problems for non-interactive installations
  7. #1376122: Stream wrappers of parent site are leaking into all tests
  8. #312458: HTML filter is not run first by default, despite default weight

Bugs related to usage of testing profile:

  1. #1181776: Change theme_default variable to Stark
  2. #1373634: Installation profile is not installed and not registered as module, unless identical to parent site
  3. #375397: Make Node module optional - Submitting the site information form with Testing profile yields error: "The path 'node' is either invalid or you do not have access to it."
  4. #1373312: Assign system_main block to 'content' region and help block to 'help' region by default (followup) - Submitting the block overview form with Testing profile yields error: "Region for Main page content block field is required."
  5. #1376150: Shortcut module installation fails in tests when installed later (due to menu system not saving menu links correctly)

»»» Action plan «««

  1. Fix most of the above listed dependencies.
  2. Create a patch that makes tests use the Testing profile by default, and adjusts/fixes tests accordingly.
    Only change tests if required. Goal: 0 failures. Almost done!
  3. Commit #375397: Make Node module optional

    Most tests throughout core implicitly depend on Node module. Thus, test failures will bump back to 6,000 or more. ;)

  4. Temporarily add the following to testing.info:
    dependencies[] = node
    
  5. Commit this patch as a first milestone.

Follow-up tasks

  1. Consider to backport this patch to D7 to also improve performance for D7 patches.

    For BC, hack DrupalWebTestCase to only use the Testing profile for Drupal core tests by default. Thus, contrib and custom modules are not affected.

  2. Separate issue: #1541298: Remove Node module dependency from Testing profile, and fix the giant mess that this will cause.
  3. Separate issues: Make not really required modules not required: Filter, Text, Field, Entity. Define proper dependencies instead.
  4. Cut the total time of running the complete test suite once more in half via #1411074: Add a flag to set up test environment only once per test class; introduce setUpBeforeClass() and tearDownAfterClass() [PHPUnit]
  5. Figure out a clean way for modules to provide a "default dummy configuration for testing": #913086: Allow modules to provide default configuration for running tests
CommentFileSizeAuthor
#84 platform.testing.84.patch126.48 KBsun
#82 platform.testing.82.patch126.38 KBsun
#78 platform.testing.78.patch129.65 KBsun
#73 drupal.testing-do-not-test.73.patch2.02 KBsun
#69 platform.testing.69.patch128.91 KBsun
#67 platform.testing.67.patch128.86 KBsun
#66 platform.testing.66.patch128.86 KBsun
#64 platform.testing.64.patch130.48 KBsun
#62 platform.testing.62.patch128.67 KBamateescu
#62 interdiff-60.txt9.26 KBamateescu
#60 platform.testing.60.patch120.93 KBsun
#58 platform.testing.58.patch112.99 KBsun
#56 drupal8.testing.56.patch114.04 KBsun
#56 interdiff.txt1.01 KBsun
#55 drupal8.testing.55.patch113.03 KBsun
#52 drupal8.testing.52.patch119.28 KBsun
#52 drupal8.testing.52.node_.patch119.61 KBsun
#51 drupal8.testing.51.patch120.87 KBsun
#47 drupal8.testing.47.patch124.86 KBsun
#43 drupal8.testing.43.patch127.74 KBsun
#42 drupal8.testing.42.patch126.59 KBsun
#38 drupal8.testing.38.patch127.36 KBAnonymous (not verified)
#33 drupal8.testing.33.patch127.01 KBAnonymous (not verified)
#29 drupal8.testing.29.patch126.47 KBsun
#27 drupal8.testing.27.patch122.71 KBsun
#25 drupal8.testing.25.patch99.95 KBsun
#24 drupal8.testing.24.patch95.93 KBsun
#21 1373142-testing-profile-21.patch82.77 KBaspilicious
#19 drupal8.testing.19.patch91.63 KBsun
#17 drupal8.testing.17.patch84.39 KBsun
#15 drupal8.testing.15.patch79.81 KBsun
#13 drupal8.testing.13.patch73.5 KBsun
#11 drupal8.testing.11.patch63.38 KBsun
#9 drupal8.testing.9.patch53.66 KBsun
#7 drupal8.testing.7.patch50.3 KBsun
#6 drupal8.testing.6.patch51.13 KBsun
#5 drupal8.testing.5.patch42.13 KBsun
#3 drupal8.testing.3.patch15.56 KBsun
drupal8.testing.0.patch489 bytessun
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

Issue tags: +API clean-up, +Testing system
sun’s picture

Issue summary: View changes

Updated issue summary.

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.0.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Status: Needs work » Needs review
FileSize
15.56 KB

FWIW, I started to adjust and fix failing tests from the top (as listed in the testbot results; i.e., sorted by test title, not group/module).

Note that the issue summary contains some new issues that need to be fixed first. This patch will contain the required fixes as long as those are not fixed.

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.3.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

sun’s picture

FileSize
51.13 KB
sun’s picture

FileSize
50.3 KB

I just recalled that we have drupalCreateContentType(), so this patch got a bit smaller.

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.7.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
53.66 KB

Last patch for today.

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.9.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
63.38 KB

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.11.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
73.5 KB

Added a major @todo to File API tests (which probably should be extracted into an issue).

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.13.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
79.81 KB

More fixes.

Duly noted: Yay! Below 1,000 failures!

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.15.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
84.39 KB

Should resolve some larger chunks.

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.17.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
91.63 KB

Way more fixes.

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.19.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review
FileSize
82.77 KB

I fixed one issue! :D yeay!

Status: Needs review » Needs work

The last submitted patch, 1373142-testing-profile-21.patch, failed testing.

aspilicious’s picture

Damnit, I just see this will be fixed by #1203886: Remove the PHP module from Drupal core :)

sun’s picture

Status: Needs work » Needs review
FileSize
95.93 KB

Resolved all User tests.

sun’s picture

FileSize
99.95 KB

Resolved all Locale tests.

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.25.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Status: Needs work » Needs review
FileSize
122.71 KB

Resolved almost all remaining tests.

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.27.patch, failed testing.

sun’s picture

Title: Use the Testing profile. Speed up tests by 900% » Use the Testing profile. Speed up testbot by 45%
Status: Needs work » Needs review
FileSize
126.47 KB

Semi-last fixes; except for the remaining, which are very hard to resolve.

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.29.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

Updated issue summary.

Anonymous’s picture

the field_ui exception stuff is a straight bug somewhere in field or field_ui. to reproduce:

1. drush -y si minimal; drush -y en field_ui
2. visit 'admin/structure/types/add', create content type
3. edit content type from 2., just change the machine name and save
4. visit the manage fields page for the content type from 2., FAIL, notice as in the test.

so yay, cutting away the bloat revealed a real bug.

Anonymous’s picture

i've created #1375452: Renaming a content type bundle causes notices on manage fields page (test improvements) to track the bug at #31.

not sure if that should be listed as a dependency or not - i'll add it for now, @sun feel free to take it out if you disagree.

Anonymous’s picture

Issue summary: View changes

Updated issue summary.

Anonymous’s picture

FileSize
127.01 KB

the testShortcutQuickLink failure from some more coupling we were hiding with bloat.

the link is only output when theme_get_setting('shortcut_module_link') is true. in core, this only seems to be true for seven. set in seven's .info. with no UI to change it. so that's number one.

second thing is that of the links we setup in the test, we check for a non-admin shortcut link. of course, this requires another setting to be true - 'node_admin_theme'.

 function testShortcutQuickLink() {
    theme_enable(array('seven'));
    variable_set('admin_theme', 'seven');
    variable_set('node_admin_theme', TRUE);
    $this->drupalGet($this->set->links[0]['link_path']);
    $this->assertRaw(t('Remove from %title shortcuts', array('%title' => $this->set->title)), '"Add to shortcuts" link p
  } 

so i've updated that test to make it pass, not sure if we want to do that in setUp().

Anonymous’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.33.patch, failed testing.

Anonymous’s picture

    // Test autocomplete on term 2, which contains a comma.
    // The term will be quoted, and the " will be encoded in unicode (\u0022).
    $input = substr($term2->name, 0, 3);
    $out = $this->drupalGet('taxonomy/autocomplete/taxonomy_' . $this->vocabulary->machine_name . '/' . $input);
    $this->assertEqual($out, '{"\u0022' . $term2->name . '\u0022":"' . $term2->name . '"}');

    // Test autocomplete on term 3 - it is alphanumeric only, so no extra
    // quoting.
    $input = substr($term3->name, 0, 3);
    $out = $this->drupalGet('taxonomy/autocomplete/taxonomy_' . $this->vocabulary->machine_name . '/' . $input);
    $this->assertEqual($out, '{"' . $term3->name . '":"' . $term3->name . '"}');

these are not the terms you are looking for. well, some of the time, just to make it more fun:

    list($term1, $term2, $term3) = array_values(taxonomy_term_load_multiple(FALSE));

the order varies with each request, so sometimes we get the term with the comma in the right spot. sometimes not. happy times.

catch’s picture

That autocomplete test just went in with the valid jain issue (on phone so no nid).

Anonymous’s picture

Status: Needs work » Needs review
FileSize
127.36 KB

updated patch fixes the failing taxonomy test to not rely on the order of terms returned by taxonomy_term_load().

we should now be down to just the notice.

chx’s picture

You guys are so incredible there are no words to describe it. I thought this will take months and you did it in three days??

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.38.patch, failed testing.

sun’s picture

yay, thanks @beejeebus for looking into those last ones! :)

I had created #1375134: Random test failure in TaxonomyTermTestCase::testNodeTermCreationAndDeletion() already (it's also in the dependencies). For me, that test locally passes 50% of the time - I'd suggest to fix it separately in that issue?

sun’s picture

Issue summary: View changes

Added node/field_ui bug to list of dependencies.

sun’s picture

Issue summary: View changes

PHP module removal is no longer a dependency.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Status: Needs work » Needs review
FileSize
126.59 KB

Removed some obsolete commented out lines that were mistakenly contained since #29.

I've also created individual issues for all bug fixes that are contained in this patch now but shouldn't. They are in the list of dependencies and most of them probably need to be backported.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.43.patch, failed testing.

xjm’s picture

In #37 I believe catch is referring to: #479368: D7: Create RFC compliant HTML safe JSON (wherein we look for valid JSON, not valid Jains). I posted a reference to that in the other issue. It didn't add the test; it just added one assertion and updated another.

Also, I'm trying very hard not to make some dumb pun about a test of faith.

There's a couple other patches out there that touch that test as well; tracking them down now.

amateescu’s picture

@xjm, one of them is #490182-2: Present all CRUD operations on Taxonomy List terms page. In the last patch from #4, I gave up on trying to add a new term and worked around it by reusing one of the three terms that were already created. For some reason, that test reported consistent results only with three terms :)

amateescu’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Status: Needs work » Needs review
FileSize
124.86 KB

Re-rolled against latest HEAD.

RobLoach’s picture

Status: Needs review » Reviewed & tested by the community

Oh man, this is absolutely huge... Get this in right now! Applies cleanly, tests pass, and tests pass fast. Is there any documentation around that we should update recommending people against building tests off the standard profile?

sun’s picture

Status: Reviewed & tested by the community » Postponed

PASSED.

Hence, this issue is postponed on fixing the dependencies first, as listed in the action plan.

That is, because I'm highly uncomfortable with squeezing fixes for bugs of varying severity into this monster.

sun’s picture

Issue summary: View changes

Updated issue summary.

Anonymous’s picture

Issue summary: View changes

Updating dependencies.

Anonymous’s picture

Issue summary: View changes

More updates to dependencies

Anonymous’s picture

Issue summary: View changes

Yay, another blocker bites the dust.

webchick’s picture

Oh, wow! This is totally exciting. :D Great work, sun!

I'm following this and the other related issues, and will try and jump on anything that applies to D7 as well.

webchick’s picture

Issue summary: View changes

put del inside of li

sun’s picture

Status: Postponed » Needs review
FileSize
120.87 KB

Re-rolled against latest HEAD. Getting smaller with every fixed dependency now :)

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Just for kicks. ;)

Status: Needs review » Needs work
Issue tags: -API clean-up, -Testing system

The last submitted patch, drupal8.testing.52.node_.patch, failed testing.

jbrown’s picture

Status: Needs work » Needs review
Issue tags: +API clean-up, +Testing system

#51: drupal8.testing.51.patch queued for re-testing.

sun’s picture

FileSize
113.03 KB

Monster rebase.

sun’s picture

FileSize
1.01 KB
114.04 KB

Added Node module dependency to Testing profile.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Issue summary: View changes

Updated issue summary.

Status: Needs review » Needs work

The last submitted patch, drupal8.testing.56.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
112.99 KB

Re-rolled #56 and rebased http://drupalcode.org/sandbox/sun/1255586.git/shortlog/refs/heads/platfo... against latest HEAD.

No other changes, so I expect the same test failures as in #56.

Status: Needs review » Needs work

The last submitted patch, platform.testing.58.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
120.93 KB

Status: Needs review » Needs work

The last submitted patch, platform.testing.60.patch, failed testing.

amateescu’s picture

Status: Needs work » Needs review
FileSize
9.26 KB
128.67 KB

Fixed some of those fails in http://drupalcode.org/sandbox/sun/1255586.git/shortlog/refs/heads/platfo....

Remaining:
- EnableDisableTestCase -> needs sun :)
- ModuleDependencyTestCase -> This is a very weird fail, because the asserted text is actually present on the page.

Edit: Wow, 20 mins for testing this patch instead of 40+. sun++

The last submitted patch, platform.testing.62.patch, failed testing.

sun’s picture

Title: Use the Testing profile. Speed up testbot by 45% » Use the Testing profile. Speed up testbot by 50%
Status: Needs review » Reviewed & tested by the community
FileSize
130.48 KB

Attached patch will pass.

If desired, it can be merged from http://drupalcode.org/sandbox/sun/1255586.git/shortlog/refs/heads/platfo...

RobLoach’s picture

Confirmed RTBC... This is a huge step forward in making Test-Driven-Development (closer to) a reality with Drupal.

RobLoach’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

FileSize
128.86 KB

Attached patch additionally converts BlockTestCase.

I also converted the CommentHelperCase base test case, which is used by 15 other test cases, but had to revert those changes, since I ran into #312458: HTML filter is not run first by default, despite default weight, and I still don't want to squeeze in functional bug fixes into this patch.

Thus, the platform-testing branch is safe for merging again.

sun’s picture

FileSize
128.86 KB

The platform-testing branch has been cleaned up (as I'm not sure whether it's going to be merged or not). Let's see whether it still applies.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, platform.testing.67.patch, failed testing.

sun’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
128.91 KB

Created a separate platform-testing-merge branch in order to not completely break all commits in follow-up branches with each rebase against 8.x.

catch’s picture

Status: Reviewed & tested by the community » Fixed

OK I had a look through, there are two non-test changes here:

1. check if the drupal install was attempted in shortcut.install, we already have at least one issue open related to that.
2. A @todo to make the testing profile not depend on node module (which is a test change really, just not in a .test file).

I'm fine with those, so I've committed/pushed this to 8.x.

This may break some existing patches in the queue, but we'll find out twice as fast :P

xjm’s picture

This may break some existing patches in the queue, but we'll find out twice as fast :P

Indeed!

webchick’s picture

Question. Anything in here that can be back-ported? That'd speed up the testbot 100% :D

sun’s picture

Version: 8.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)
Issue tags: +Needs backport to D7
FileSize
2.02 KB

Yes, I actually think we can. Suggested in the OP already:

Consider to backport this patch to D7 to also improve performance for D7 patches.

For BC, hack DrupalWebTestCase to only use the Testing profile for Drupal core tests by default. Thus, contrib and custom modules are not affected.

Attached patch shows how this could work.

Albert Volkman’s picture

Status: Patch (to be ported) » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal.testing-do-not-test.73.patch, failed testing.

Albert Volkman’s picture

Sorry about that.

scito’s picture

Status: Needs work » Patch (to be ported)

Set previous state.

sun’s picture

Status: Patch (to be ported) » Needs review
FileSize
129.65 KB

Backport to D7, including #73.

EDIT: See platform-d7-testing branch.

Status: Needs review » Needs work

The last submitted patch, platform.testing.78.patch, failed testing.

chx’s picture

No need to use DIRECTORY_SEPARATOR / works on Windows too and it's used throughout in core.

Tor Arne Thune’s picture

+++ b/modules/block/block.testundefined
@@ -776,6 +821,10 @@ class BlockHiddenRegionTestCase extends DrupalWebTestCase {
+    $this->assertText('Search', t('Block was displayed on the front page.'));

This needs to assert for 'Search form' in D7, as later in the D7 test.

#1285364: Remove search-block-form template for form rendering consistency removed the template for the search block in D8, which caused the block to output just its block title ('Search') instead of the title set in the removed template ('Search form') for the search block. That's why the D7 and D8 test asserts for different text.

sun’s picture

Status: Needs work » Needs review
FileSize
126.38 KB

#1373312: Assign system_main block to 'content' region and help block to 'help' region by default (followup) is a hard dependency for this backport.

Attached patch contains it, which should make most tests pass.

Additionally, attached patch "resets" the default theme to Stark in testing.install, so tests that use the Testing profile do not test against Bartik. While that results in a pretty stupid back and forth of theme switching during Drupal installation with Testing profile, it's the only way to make tests run against the clean, unmodified module output without any theme adjustments or overrides.

re #80: DIRECTORY_SEPARATOR is used here, since the filepath in $caller['file'] is not generated by Drupal, but by PHP's native ReflectionMethod->getFileName(). It contains the full path to the file that contains the test method and uses backslashes as directory separators on Windows. Since we attempt to compare this path to the path of getcwd() (equally using backslashes on Windows) plus modules, we need to build the comparison string with the appropriate directory separator. I'm on Windows myself, which is why I encountered the issue in the first place and why had to use DIRECTORY_SEPARATOR.

Status: Needs review » Needs work

The last submitted patch, platform.testing.82.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
126.48 KB

Rebased against latest 7.x.

Status: Needs review » Needs work

The last submitted patch, platform.testing.84.patch, failed testing.

xjm’s picture

xjm’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Priority: Major » Normal
sun’s picture

Assigned: sun » Unassigned
sun’s picture

Version: 7.x-dev » 8.x-dev
Status: Needs work » Closed (fixed)
webchick’s picture

Doesn't look like a change notice was ever added for this so I made http://drupal.org/node/1911318.

sun’s picture

I've slightly adjusted the title of that change notice, since "minimal testing profile" was slightly too ambiguous ;)

pfrenssen’s picture

This has not yet been backported to D7. This has been closed without comment in #89, was this intentional?

pfrenssen’s picture

Issue summary: View changes

Updated issue summary.