Posted by sun on December 16, 2011 at 4:36pm
23 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | simpletest.module |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | API clean-up, needs backport to D7, Testing system |
Issue Summary
- 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.
- It significantly speeds up test runs.
- It should be used by default.
- 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.
- Other Testing system issues I'm working on will speed up the Testing profile even further.
Dependencies
Functional bugs:
#1375134: Random test failure in TaxonomyTermTestCase::testNodeTermCreationAndDeletion()#1375452: Renaming a content type bundle causes notices on manage fields page (test improvements)#1373360: Fatal error if optional Block module is not enabled#1376164: Format SimpleTestFunctionalTest description according to standards#1376166: Custom logo and favicon functionality inanely tries to support absolute local file paths- #1213536: Non-resettable theme_get_registry() cache causes problems for non-interactive installations
- #1376122: Tests are executed in incomplete environment (was: Stream wrappers not registered when calling file_unmanaged_*())
- #312458: HTML filter is not run first by default, despite default weight
Bugs related to usage of testing profile:
#1181776: Change theme_default variable to Stark#1373634: Installation profile is not installed and not registered as module, unless identical to parent site#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."- #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."
- #1376150: Shortcut module installation fails in tests when installed later (due to menu system not saving menu links correctly)
»»» Action plan «««
Fix most of the above listed dependencies.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!Commit #375397: Make Node module optionalMost tests throughout core implicitly depend on Node module. Thus, test failures will bump back to 6,000 or more. ;)
Temporarily add the following totesting.info:dependencies[] = node- Commit this patch as a first milestone.
Follow-up tasks
- 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.
- Separate issue: #1541298: Remove Node module dependency from Testing profile, and fix the giant mess that this will cause.
- Separate issues: Make not really required modules not required: Filter, Text, Field, Entity. Define proper dependencies instead.
- Cut the total time of running the complete test suite once more in half via #1411074: Setup test environment only once per test class; introduce setUpBeforeClass() and tearDownAfterClass() [PHPUnit]
- 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
Change records for this issue
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| drupal8.testing.0.patch | 489 bytes | Idle | FAILED: [[SimpleTest]]: [MySQL] 25,745 pass(es), 6,582 fail(s), and 3,414 exception(es). | View details |
Comments
#1
#2
The last submitted patch, drupal8.testing.0.patch, failed testing.
#3
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.
#4
The last submitted patch, drupal8.testing.3.patch, failed testing.
#5
#1373634: Installation profile is not installed and not registered as module, unless identical to parent site explains a lot. :(
#6
#7
I just recalled that we have drupalCreateContentType(), so this patch got a bit smaller.
#8
The last submitted patch, drupal8.testing.7.patch, failed testing.
#9
Last patch for today.
#10
The last submitted patch, drupal8.testing.9.patch, failed testing.
#11
#12
The last submitted patch, drupal8.testing.11.patch, failed testing.
#13
Added a major @todo to File API tests (which probably should be extracted into an issue).
#14
The last submitted patch, drupal8.testing.13.patch, failed testing.
#15
More fixes.
Duly noted: Yay! Below 1,000 failures!
#16
The last submitted patch, drupal8.testing.15.patch, failed testing.
#17
Should resolve some larger chunks.
#18
The last submitted patch, drupal8.testing.17.patch, failed testing.
#19
Way more fixes.
#20
The last submitted patch, drupal8.testing.19.patch, failed testing.
#21
I fixed one issue! :D yeay!
#22
The last submitted patch, 1373142-testing-profile-21.patch, failed testing.
#23
Damnit, I just see this will be fixed by #1203886: Split the PHP text filter out of the main PHP module (and consider removing the filter from core later) :)
#24
Resolved all User tests.
#25
Resolved all Locale tests.
#26
The last submitted patch, drupal8.testing.25.patch, failed testing.
#27
Resolved almost all remaining tests.
#28
The last submitted patch, drupal8.testing.27.patch, failed testing.
#29
Semi-last fixes; except for the remaining, which are very hard to resolve.
#30
The last submitted patch, drupal8.testing.29.patch, failed testing.
#31
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.
#32
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.
#33
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'.
<?phpfunction 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().
#34
#35
The last submitted patch, drupal8.testing.33.patch, failed testing.
#36
<?php
// 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:
<?phplist($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.
#37
That autocomplete test just went in with the valid jain issue (on phone so no nid).
#38
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.
#39
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??
#40
The last submitted patch, drupal8.testing.38.patch, failed testing.
#41
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?
#42
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.
#43
Just for kicks, incorporated fix from #1375452: Renaming a content type bundle causes notices on manage fields page (test improvements)
#44
The last submitted patch, drupal8.testing.43.patch, failed testing.
#45
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.
#46
@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 :)
#47
Re-rolled against latest HEAD.
#48
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?
#49
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.
#50
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.
#51
Re-rolled against latest HEAD. Getting smaller with every fixed dependency now :)
#52
Just for kicks. ;)
#53
The last submitted patch, drupal8.testing.52.node_.patch, failed testing.
#54
#51: drupal8.testing.51.patch queued for re-testing.
#55
Monster rebase.
#56
Added Node module dependency to Testing profile.
#57
The last submitted patch, drupal8.testing.56.patch, failed testing.
#58
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.
#59
The last submitted patch, platform.testing.58.patch, failed testing.
#60
#61
The last submitted patch, platform.testing.60.patch, failed testing.
#62
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++
#63
The last submitted patch, platform.testing.62.patch, failed testing.
#64
Attached patch will pass.
If desired, it can be merged from http://drupalcode.org/sandbox/sun/1255586.git/shortlog/refs/heads/platfo...
#65
Confirmed RTBC... This is a huge step forward in making Test-Driven-Development (closer to) a reality with Drupal.
#66
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.
#67
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.
#68
The last submitted patch, platform.testing.67.patch, failed testing.
#69
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.
#70
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
#71
Indeed!
#72
Question. Anything in here that can be back-ported? That'd speed up the testbot 100% :D
#73
Yes, I actually think we can. Suggested in the OP already:
Attached patch shows how this could work.
#74
#75
The last submitted patch, drupal.testing-do-not-test.73.patch, failed testing.
#76
Sorry about that.
#77
Set previous state.
#78
Backport to D7, including #73.
EDIT: See platform-d7-testing branch.
#79
The last submitted patch, platform.testing.78.patch, failed testing.
#80
No need to use DIRECTORY_SEPARATOR / works on Windows too and it's used throughout in core.
#81
+++ b/modules/block/block.testundefined@@ -776,6 +821,10 @@ class BlockHiddenRegionTestCase extends DrupalWebTestCase {
+ $this->assertText('Search', t('Block was displayed on the front page.'));
#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.
#82
#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 ofgetcwd()(equally using backslashes on Windows) plusmodules, 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.#83
The last submitted patch, platform.testing.82.patch, failed testing.
#84
Rebased against latest 7.x.
#85
The last submitted patch, platform.testing.84.patch, failed testing.
#86
#1302228-48: field_has_data() returns inconsistent results – possible data loss adds
NodeWebTestCaseto D7.#87
#88
#89
#90
Doesn't look like a change notice was ever added for this so I made http://drupal.org/node/1911318.
#91
I've slightly adjusted the title of that change notice, since "minimal testing profile" was slightly too ambiguous ;)