- 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: Stream wrappers of parent site are leaking into all tests
- #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: Add a flag to set up 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
Comment | File | Size | Author |
---|---|---|---|
#84 | platform.testing.84.patch | 126.48 KB | sun |
#82 | platform.testing.82.patch | 126.38 KB | sun |
#78 | platform.testing.78.patch | 129.65 KB | sun |
#73 | drupal.testing-do-not-test.73.patch | 2.02 KB | sun |
#69 | platform.testing.69.patch | 128.91 KB | sun |
Comments
Comment #1
sunComment #1.0
sunUpdated issue summary.
Comment #2.0
(not verified) CreditAttribution: commentedUpdated issue summary.
Comment #2.1
sunUpdated issue summary.
Comment #2.2
sunUpdated issue summary.
Comment #3
sunFWIW, 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.
Comment #4.0
(not verified) CreditAttribution: commentedUpdated issue summary.
Comment #4.1
sunUpdated issue summary.
Comment #4.2
sunUpdated issue summary.
Comment #5
sun#1373634: Installation profile is not installed and not registered as module, unless identical to parent site explains a lot. :(
Comment #6
sunComment #7
sunI just recalled that we have drupalCreateContentType(), so this patch got a bit smaller.
Comment #9
sunLast patch for today.
Comment #11
sunComment #13
sunAdded a major @todo to File API tests (which probably should be extracted into an issue).
Comment #15
sunMore fixes.
Duly noted: Yay! Below 1,000 failures!
Comment #17
sunShould resolve some larger chunks.
Comment #19
sunWay more fixes.
Comment #21
aspilicious CreditAttribution: aspilicious commentedI fixed one issue! :D yeay!
Comment #23
aspilicious CreditAttribution: aspilicious commentedDamnit, I just see this will be fixed by #1203886: Remove the PHP module from Drupal core :)
Comment #24
sunResolved all User tests.
Comment #25
sunResolved all Locale tests.
Comment #26.0
(not verified) CreditAttribution: commentedUpdated issue summary.
Comment #27
sunResolved almost all remaining tests.
Comment #29
sunSemi-last fixes; except for the remaining, which are very hard to resolve.
Comment #30.0
(not verified) CreditAttribution: commentedUpdated issue summary.
Comment #31
Anonymous (not verified) CreditAttribution: Anonymous commentedthe 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.
Comment #32
Anonymous (not verified) CreditAttribution: Anonymous commentedi'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.
Comment #32.0
Anonymous (not verified) CreditAttribution: Anonymous commentedUpdated issue summary.
Comment #33
Anonymous (not verified) CreditAttribution: Anonymous commentedthe 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'.
so i've updated that test to make it pass, not sure if we want to do that in setUp().
Comment #34
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #36
Anonymous (not verified) CreditAttribution: Anonymous commentedthese are not the terms you are looking for. well, some of the time, just to make it more fun:
the order varies with each request, so sometimes we get the term with the comma in the right spot. sometimes not. happy times.
Comment #37
catchThat autocomplete test just went in with the valid jain issue (on phone so no nid).
Comment #38
Anonymous (not verified) CreditAttribution: Anonymous commentedupdated 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.
Comment #39
chx CreditAttribution: chx commentedYou guys are so incredible there are no words to describe it. I thought this will take months and you did it in three days??
Comment #41
sunyay, 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?
Comment #41.0
sunAdded node/field_ui bug to list of dependencies.
Comment #41.1
sunPHP module removal is no longer a dependency.
Comment #41.2
sunUpdated issue summary.
Comment #41.3
sunUpdated issue summary.
Comment #41.4
sunUpdated issue summary.
Comment #42
sunRemoved 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.
Comment #42.0
sunUpdated issue summary.
Comment #43
sunJust for kicks, incorporated fix from #1375452: Renaming a content type bundle causes notices on manage fields page (test improvements)
Comment #45
xjmIn #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.
Comment #46
amateescu CreditAttribution: amateescu commented@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 :)
Comment #46.0
amateescu CreditAttribution: amateescu commentedUpdated issue summary.
Comment #47
sunRe-rolled against latest HEAD.
Comment #48
RobLoachOh 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?
Comment #49
sunPASSED.
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.
Comment #49.0
sunUpdated issue summary.
Comment #49.1
Anonymous (not verified) CreditAttribution: Anonymous commentedUpdating dependencies.
Comment #49.2
Anonymous (not verified) CreditAttribution: Anonymous commentedMore updates to dependencies
Comment #49.3
Anonymous (not verified) CreditAttribution: Anonymous commentedYay, another blocker bites the dust.
Comment #50
webchickOh, 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.
Comment #50.0
webchickput del inside of li
Comment #51
sunRe-rolled against latest HEAD. Getting smaller with every fixed dependency now :)
Comment #51.0
sunUpdated issue summary.
Comment #51.1
sunUpdated issue summary.
Comment #51.2
sunUpdated issue summary.
Comment #51.3
sunUpdated issue summary.
Comment #52
sunJust for kicks. ;)
Comment #54
jbrown CreditAttribution: jbrown commented#51: drupal8.testing.51.patch queued for re-testing.
Comment #55
sunMonster rebase.
Comment #56
sunAdded Node module dependency to Testing profile.
Comment #56.0
sunUpdated issue summary.
Comment #56.1
sunUpdated issue summary.
Comment #56.2
sunUpdated issue summary.
Comment #58
sunRe-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.
Comment #60
sunComment #62
amateescu CreditAttribution: amateescu commentedFixed 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++
Comment #64
sunAttached patch will pass.
If desired, it can be merged from http://drupalcode.org/sandbox/sun/1255586.git/shortlog/refs/heads/platfo...
Comment #65
RobLoachConfirmed RTBC... This is a huge step forward in making Test-Driven-Development (closer to) a reality with Drupal.
Comment #65.0
RobLoachUpdated issue summary.
Comment #65.1
sunUpdated issue summary.
Comment #65.2
sunUpdated issue summary.
Comment #66
sunAttached 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.
Comment #67
sunThe 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.
Comment #69
sunCreated a separate platform-testing-merge branch in order to not completely break all commits in follow-up branches with each rebase against 8.x.
Comment #70
catchOK 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
Comment #71
xjmIndeed!
Comment #72
webchickQuestion. Anything in here that can be back-ported? That'd speed up the testbot 100% :D
Comment #73
sunYes, I actually think we can. Suggested in the OP already:
Attached patch shows how this could work.
Comment #74
Albert Volkman CreditAttribution: Albert Volkman commentedComment #76
Albert Volkman CreditAttribution: Albert Volkman commentedSorry about that.
Comment #77
scitoSet previous state.
Comment #78
sunBackport to D7, including #73.
EDIT: See platform-d7-testing branch.
Comment #80
chx CreditAttribution: chx commentedNo need to use DIRECTORY_SEPARATOR / works on Windows too and it's used throughout in core.
Comment #81
Tor Arne Thune CreditAttribution: Tor Arne Thune commentedThis 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.
Comment #82
sun#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.Comment #84
sunRebased against latest 7.x.
Comment #86
xjm#1302228-48: field_has_data() returns inconsistent results – possible data loss adds
NodeWebTestCase
to D7.Comment #86.0
xjmUpdated issue summary.
Comment #87
sunComment #88
sunComment #89
sunComment #90
webchickDoesn't look like a change notice was ever added for this so I made http://drupal.org/node/1911318.
Comment #91
sunI've slightly adjusted the title of that change notice, since "minimal testing profile" was slightly too ambiguous ;)
Comment #92
pfrenssenThis has not yet been backported to D7. This has been closed without comment in #89, was this intentional?
Comment #92.0
pfrenssenUpdated issue summary.