Task

Create a Twig template to replace the theme function theme_simpletest_result_summary().

Remaining

  • Patch needs review
  • Manual testing

Theme function name/template path Conversion status
theme_simpletest_result_summary converted

Testing steps

Compare markup and test functionality for the following:

  1. Enable Testing module (simpletest.module).
  2. Navigate to admin/config/development/testing.
  3. Select two or three individual tests to run (expand to choose individual tests). - this table is simpletest-test-table.html.twig (the table theme function was converted to #type table instead)
  4. Wait for the test results to come back. - this table uses simpletest-result-summary.html.twig.

Run another test in the command line.
Example test run.
php ./core/scripts/run-tests.sh --url http://d8.dev --verbose --color --class "Drupal\system\Tests\Theme\TwigFilterTest"

#1757550: [Meta] Convert core theme functions to Twig templates

Files: 
CommentFileSizeAuthor
#101 interdiff.txt1.96 KBCottser
#101 1898452-101.patch5.9 KBCottser
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,838 pass(es). View

Comments

c4rl’s picture

Issue tags: +Twig

Tagging

disasm’s picture

Status: Active » Needs review
FileSize
3.54 KB
FAILED: [[SimpleTest]]: [MySQL] 52,061 pass(es), 72 fail(s), and 84 exception(s). View

Attached is a super hacky patch. For now, just setting the themed table as a variable table and outputting that variable by itself in the theme. If this does go green by some miracle, need to clean up template_preprocess_simpletest_test_table and it's twig template.

Status: Needs review » Needs work

The last submitted patch, drupal-simpletest_twig-1898452-2.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
4.61 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site. View
4 KB

A little less rough;)

joelpittet’s picture

Assigned: Unassigned » joelpittet

Status: Needs review » Needs work
Issue tags: -Twig

The last submitted patch, drupal-simpletest_twig-1898452-4.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal-simpletest_twig-1898452-4.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +Twig

The last submitted patch, drupal-simpletest_twig-1898452-4.patch, failed testing.

Cottser’s picture

+++ b/.htaccessundefined
@@ -107,7 +107,7 @@ DirectoryIndex index.php index.html index.htm
-  # RewriteBase /
+  RewriteBase /

I'm thinking this is probably why testbot can't login to the test site.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
4.19 KB
FAILED: [[SimpleTest]]: [MySQL] 52,204 pass(es), 23 fail(s), and 42 exception(s). View

Oh that's not a good thing to have in a commit:S

Same as #4 but without the Rebase change from my dev env.

Status: Needs review » Needs work

The last submitted patch, drupal-simpletest_twig-1898452-12.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
1 KB
4.54 KB
FAILED: [[SimpleTest]]: [MySQL] 52,619 pass(es), 23 fail(s), and 42 exception(s). View

I have a feeling this may still fail testbot but it's slightly closer than it was. I converted the '#theme' => 'table'
to
'#type' => 'table'

Which does good for the first table but the results after the tests are messed up and I have a feeling it has to do with colspan issue noted in http://drupal.org/node/1876712#comment-7156742.

#1876712: [meta] Convert all tables in core to new #type 'table'

Status: Needs review » Needs work

The last submitted patch, drupal-simpletest_twig-1898452-14.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
5.08 KB
PASSED: [[SimpleTest]]: [MySQL] 53,035 pass(es). View
2.36 KB

there was a nasty typo and a issue with variables vs render element

Cottser’s picture

Issue tags: +Needs manual testing

Tagging.

Cottser’s picture

Issue summary: View changes

Add conversion summary table

Cottser’s picture

Issue summary: View changes

Add testing steps

Cottser’s picture

Issue tags: -Needs manual testing

I just tested this patch, works perfectly!

+++ b/core/modules/simpletest/simpletest.pages.incundefined
@@ -58,7 +58,7 @@ function simpletest_test_form($form, &$form_state) {
+ * Preprocess variables for a test list generated by simpletest_test_form() into a table.
+++ b/core/modules/simpletest/simpletest.pages.incundefined
@@ -370,7 +371,7 @@ function simpletest_result_form_submit($form, &$form_state) {
+ * Preprocess variables for the summary status of a simpletest result.

The preprocess function docblocks should be updated to meet #1913208: [policy] Standardize template preprocess function documentation:

Prepares variables for test list templates. / Prepares variables for test result summary templates.

Default template: …

@param array $variables

And remove @ingroup themeable.

Once that's done, I think this is ready to go.

Cottser’s picture

Or maybe "test list table templates". Just suggestions :)

Cottser’s picture

Status: Needs review » Needs work

CNW for #18.

joelpittet’s picture

FileSize
1.5 KB
5.18 KB
PASSED: [[SimpleTest]]: [MySQL] 54,267 pass(es). View
joelpittet’s picture

Status: Needs work » Needs review
FileSize
2.3 KB
6.37 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/simpletest/simpletest.pages.inc. View

Did a bit more, and fixed the typo... Could do table twig template or possibly convert to type table.

Status: Needs review » Needs work

The last submitted patch, twig-simpletest-1898452-22.patch, failed testing.

Cottser’s picture

The changes are looking good (other than the syntax bit), will leave this at needs review for now, @joelpittet said he would take another look at the table again to see what's possible.

+++ b/core/modules/simpletest/simpletest.pages.incundefined
@@ -373,7 +387,7 @@ function simpletest_result_form_submit($form, &$form_state) {
  * Prepares variables for the summary status of a simpletest result.

This should end in "templates".

joelpittet’s picture

Status: Needs work » Needs review
FileSize
5.18 KB
PASSED: [[SimpleTest]]: [MySQL] 54,269 pass(es). View
1.88 KB

Well, got crazy there... those need to be rendered with theme( the way they are because they are used for JS.

But that style should really be done with CSS... instead of images.

Reverted and added doc cleanup.

joelpittet’s picture

FileSize
10.31 KB
PASSED: [[SimpleTest]]: [MySQL] 54,294 pass(es). View
166.43 KB
PASSED: [[SimpleTest]]: [MySQL] 54,415 pass(es). View
joelpittet’s picture

FileSize
10.31 KB
PASSED: [[SimpleTest]]: [MySQL] 54,537 pass(es). View
7.31 KB

hmm one of those should be an interdiff. The save button got away on me before I could remove the non-rebased one.

Ok so, I replaced the theme_table with a twig table, in hopes in the future some brave soul would find it easier to swap the table out with some dl/ol/ul markup. Zebra was being added (but not used) and affected the layout so I added a css class to keep the margin down as in the original.

joelpittet’s picture

Status: Needs review » Needs work
+++ b/core/modules/simpletest/simpletest.pages.inc
@@ -81,22 +81,8 @@ function template_preprocess_simpletest_test_table(&$variables) {
+      theme('image', array('uri' => 'core/misc/menu-collapsed.png', 'width' => 7, 'height' => 7, 'alt' => t('Expand'), 'title' => t('Expand'))) . ' <a href="#" class="simpletest-collapse">(' . t('Expand') . ')</a>',
+      theme('image', array('uri' => 'core/misc/menu-expanded.png', 'width' => 7, 'height' => 7, 'alt' => t('Collapse'), 'title' => t('Collapse'))) . ' <a href="#" class="simpletest-collapse">(' . t('Collapse') . ')</a>',

Can't think of a good way to remove these theme() without losing the localization.

joelpittet’s picture

Status: Needs work » Needs review
Issue tags: +Needs manual testing

Ok we are going to leave that theme('image') and use #27 as discussed with Cottser. This should be probably reworked for the CSS/JS but outside of this conversions scope. The theme('image') string is passed along to JS directly so converting to array('#theme' => 'image') won't do the trick here unless we wrap it in render() which is just busy work for people later.

Hope someone could confirm the output is right through manual testing?

thedavidmeister’s picture

I'll do the manual testing.

thedavidmeister’s picture

Issue summary: View changes

Remove empty

  • thedavidmeister’s picture

    Status: Needs review » Needs work

    This is for simpletest-test-table.html.twig for the first couple of test groups, I removed the whitespace as it was making the diff hard to read:

    Original:

    <table id="simpletest-form-table" class="sticky-enabled responsive-enabled"><thead><tr><th class="select-all"></th><th class="simpletest_test">Test</th><th class="simpletest_description">Description</th> </tr></thead><tbody><tr class="simpletest-group odd"><td id="action" class="simpletest-select-all"></td><td class="simpletest-group-label"><div class="simpletest-image" id="simpletest-test-group-action"></div><label for="action-select-all" class="simpletest-group-label">Action</label></td><td class="simpletest-group-description">&nbsp;</td> </tr><tr class="action-test js-hide even"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\action\Tests\ConfigurationTest"><label class="element-invisible" for="edit-drupalactiontestsconfigurationtest">Actions configuration</label> <input aria-describedby="edit-drupalactiontestsconfigurationtest--description" type="checkbox" id="edit-drupalactiontestsconfigurationtest" name="Drupal\action\Tests\ConfigurationTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalactiontestsconfigurationtest">Actions configuration</label></td><td class="simpletest-test-description"><div class="description">Tests complex actions configuration by adding, editing, and deleting a complex action.</div></td> </tr><tr class="action-test js-hide odd"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\action\Tests\LoopTest"><label class="element-invisible" for="edit-drupalactiontestslooptest">Actions executing in a potentially infinite loop</label> <input aria-describedby="edit-drupalactiontestslooptest--description" type="checkbox" id="edit-drupalactiontestslooptest" name="Drupal\action\Tests\LoopTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalactiontestslooptest">Actions executing in a potentially infinite loop</label></td><td class="simpletest-test-description"><div class="description">Tests actions executing in a loop, and makes sure they abort properly.</div></td> </tr><tr class="action-test js-hide even"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\action\Tests\BulkFormTest"><label class="element-invisible" for="edit-drupalactiontestsbulkformtest">Bulk form</label> <input aria-describedby="edit-drupalactiontestsbulkformtest--description" type="checkbox" id="edit-drupalactiontestsbulkformtest" name="Drupal\action\Tests\BulkFormTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalactiontestsbulkformtest">Bulk form</label></td><td class="simpletest-test-description"><div class="description">Tests the views bulk form test.</div></td> </tr><tr class="simpletest-group odd"><td id="aggregator" class="simpletest-select-all"></td><td class="simpletest-group-label"><div class="simpletest-image" id="simpletest-test-group-aggregator"></div><label for="aggregator-select-all" class="simpletest-group-label">Aggregator</label></td><td class="simpletest-group-description">&nbsp;</td> </tr><tr class="aggregator-test js-hide even"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\AddFeedTest"><label class="element-invisible" for="edit-drupalaggregatortestsaddfeedtest">Add feed functionality</label> <input aria-describedby="edit-drupalaggregatortestsaddfeedtest--description" type="checkbox" id="edit-drupalaggregatortestsaddfeedtest" name="Drupal\aggregator\Tests\AddFeedTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsaddfeedtest">Add feed functionality</label></td><td class="simpletest-test-description"><div class="description">Add feed test.</div></td> </tr><tr class="aggregator-test js-hide odd"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\AggregatorConfigurationTest"><label class="element-invisible" for="edit-drupalaggregatortestsaggregatorconfigurationtest">Aggregator configuration</label> <input aria-describedby="edit-drupalaggregatortestsaggregatorconfigurationtest--description" type="checkbox" id="edit-drupalaggregatortestsaggregatorconfigurationtest" name="Drupal\aggregator\Tests\AggregatorConfigurationTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsaggregatorconfigurationtest">Aggregator configuration</label></td><td class="simpletest-test-description"><div class="description">Test aggregator settings page.</div></td> </tr><tr class="aggregator-test js-hide even"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\CategorizeFeedTest"><label class="element-invisible" for="edit-drupalaggregatortestscategorizefeedtest">Categorize feed functionality</label> <input aria-describedby="edit-drupalaggregatortestscategorizefeedtest--description" type="checkbox" id="edit-drupalaggregatortestscategorizefeedtest" name="Drupal\aggregator\Tests\CategorizeFeedTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestscategorizefeedtest">Categorize feed functionality</label></td><td class="simpletest-test-description"><div class="description">Categorize feed test.</div></td> </tr><tr class="aggregator-test js-hide odd"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\CategorizeFeedItemTest"><label class="element-invisible" for="edit-drupalaggregatortestscategorizefeeditemtest">Categorize feed item functionality</label> <input aria-describedby="edit-drupalaggregatortestscategorizefeeditemtest--description" type="checkbox" id="edit-drupalaggregatortestscategorizefeeditemtest" name="Drupal\aggregator\Tests\CategorizeFeedItemTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestscategorizefeeditemtest">Categorize feed item functionality</label></td><td class="simpletest-test-description"><div class="description">Test feed item categorization.</div></td> </tr><tr class="aggregator-test js-hide even"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\AggregatorRenderingTest"><label class="element-invisible" for="edit-drupalaggregatortestsaggregatorrenderingtest">Checks display of aggregator items</label> <input aria-describedby="edit-drupalaggregatortestsaggregatorrenderingtest--description" type="checkbox" id="edit-drupalaggregatortestsaggregatorrenderingtest" name="Drupal\aggregator\Tests\AggregatorRenderingTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsaggregatorrenderingtest">Checks display of aggregator items</label></td><td class="simpletest-test-description"><div class="description">Checks display of aggregator items on the page.</div></td> </tr><tr class="aggregator-test js-hide odd"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\FeedFetcherPluginTest"><label class="element-invisible" for="edit-drupalaggregatortestsfeedfetcherplugintest">Feed fetcher plugins</label> <input aria-describedby="edit-drupalaggregatortestsfeedfetcherplugintest--description" type="checkbox" id="edit-drupalaggregatortestsfeedfetcherplugintest" name="Drupal\aggregator\Tests\FeedFetcherPluginTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsfeedfetcherplugintest">Feed fetcher plugins</label></td><td class="simpletest-test-description"><div class="description">Test the fetcher plugins functionality and discoverability.</div></td> </tr><tr class="aggregator-test js-hide even"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\FeedParserTest"><label class="element-invisible" for="edit-drupalaggregatortestsfeedparsertest">Feed parser functionality</label> <input aria-describedby="edit-drupalaggregatortestsfeedparsertest--description" type="checkbox" id="edit-drupalaggregatortestsfeedparsertest" name="Drupal\aggregator\Tests\FeedParserTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsfeedparsertest">Feed parser functionality</label></td><td class="simpletest-test-description"><div class="description">Test the built-in feed parser with valid feed samples.</div></td> </tr><tr class="aggregator-test js-hide odd"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\FeedProcessorPluginTest"><label class="element-invisible" for="edit-drupalaggregatortestsfeedprocessorplugintest">Feed processor plugins</label> <input aria-describedby="edit-drupalaggregatortestsfeedprocessorplugintest--description" type="checkbox" id="edit-drupalaggregatortestsfeedprocessorplugintest" name="Drupal\aggregator\Tests\FeedProcessorPluginTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsfeedprocessorplugintest">Feed processor plugins</label></td><td class="simpletest-test-description"><div class="description">Test the processor plugins functionality and discoverability.</div></td> </tr><tr class="aggregator-test js-hide even"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\ImportOpmlTest"><label class="element-invisible" for="edit-drupalaggregatortestsimportopmltest">Import feeds from OPML functionality</label> <input aria-describedby="edit-drupalaggregatortestsimportopmltest--description" type="checkbox" id="edit-drupalaggregatortestsimportopmltest" name="Drupal\aggregator\Tests\ImportOpmlTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsimportopmltest">Import feeds from OPML functionality</label></td><td class="simpletest-test-description"><div class="description">Test OPML import.</div></td> </tr><tr class="aggregator-test js-hide odd"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\FeedLanguageTest"><label class="element-invisible" for="edit-drupalaggregatortestsfeedlanguagetest">Multilingual feeds</label> <input aria-describedby="edit-drupalaggregatortestsfeedlanguagetest--description" type="checkbox" id="edit-drupalaggregatortestsfeedlanguagetest" name="Drupal\aggregator\Tests\FeedLanguageTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsfeedlanguagetest">Multilingual feeds</label></td><td class="simpletest-test-description"><div class="description">Checks creating of feeds in multiple languages</div></td> </tr><tr class="aggregator-test js-hide even"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\RemoveFeedTest"><label class="element-invisible" for="edit-drupalaggregatortestsremovefeedtest">Remove feed functionality</label> <input aria-describedby="edit-drupalaggregatortestsremovefeedtest--description" type="checkbox" id="edit-drupalaggregatortestsremovefeedtest" name="Drupal\aggregator\Tests\RemoveFeedTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsremovefeedtest">Remove feed functionality</label></td><td class="simpletest-test-description"><div class="description">Remove feed test.</div></td> </tr><tr class="aggregator-test js-hide odd"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\RemoveFeedItemTest"><label class="element-invisible" for="edit-drupalaggregatortestsremovefeeditemtest">Remove feed item functionality</label> <input aria-describedby="edit-drupalaggregatortestsremovefeeditemtest--description" type="checkbox" id="edit-drupalaggregatortestsremovefeeditemtest" name="Drupal\aggregator\Tests\RemoveFeedItemTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsremovefeeditemtest">Remove feed item functionality</label></td><td class="simpletest-test-description"><div class="description">Remove feed items from a feed.</div></td> </tr><tr class="aggregator-test js-hide even"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\UpdateFeedTest"><label class="element-invisible" for="edit-drupalaggregatortestsupdatefeedtest">Update feed functionality</label> <input aria-describedby="edit-drupalaggregatortestsupdatefeedtest--description" type="checkbox" id="edit-drupalaggregatortestsupdatefeedtest" name="Drupal\aggregator\Tests\UpdateFeedTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsupdatefeedtest">Update feed functionality</label></td><td class="simpletest-test-description"><div class="description">Update feed test.</div></td> </tr><tr class="aggregator-test js-hide odd"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\UpdateFeedItemTest"><label class="element-invisible" for="edit-drupalaggregatortestsupdatefeeditemtest">Update feed item functionality</label> <input aria-describedby="edit-drupalaggregatortestsupdatefeeditemtest--description" type="checkbox" id="edit-drupalaggregatortestsupdatefeeditemtest" name="Drupal\aggregator\Tests\UpdateFeedItemTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsupdatefeeditemtest">Update feed item functionality</label></td><td class="simpletest-test-description"><div class="description">Update feed items from a feed.</div></td> </tr><tr class="aggregator-test js-hide even"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\AggregatorCronTest"><label class="element-invisible" for="edit-drupalaggregatortestsaggregatorcrontest">Update on cron functionality</label> <input aria-describedby="edit-drupalaggregatortestsaggregatorcrontest--description" type="checkbox" id="edit-drupalaggregatortestsaggregatorcrontest" name="Drupal\aggregator\Tests\AggregatorCronTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsaggregatorcrontest">Update on cron functionality</label></td><td class="simpletest-test-description"><div class="description">Update feeds on cron.</div></td> </tr>
    

    New:

    <table id="simpletest-form-table"><thead><tr><th class="select-all"></th><th class="simpletest_test">Test</th><th class="simpletest_description">Description</th></tr></thead><tbody><tr class="simpletest-group"><td id="action" class="simpletest-select-all"></td><td class="simpletest-group-label"><div class="simpletest-image" id="simpletest-test-group-action"></div><label for="action-select-all" class="simpletest-group-label">Action</label></td><td class="simpletest-group-description">&nbsp;</td></tr><tr class="action-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\action\Tests\ConfigurationTest"><label class="element-invisible" for="edit-drupalactiontestsconfigurationtest">Actions configuration</label> <input aria-describedby="edit-drupalactiontestsconfigurationtest--description" type="checkbox" id="edit-drupalactiontestsconfigurationtest" name="Drupal\action\Tests\ConfigurationTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalactiontestsconfigurationtest">Actions configuration</label></td><td class="simpletest-test-description"><div class="description">Tests complex actions configuration by adding, editing, and deleting a complex action.</div></td></tr><tr class="action-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\action\Tests\LoopTest"><label class="element-invisible" for="edit-drupalactiontestslooptest">Actions executing in a potentially infinite loop</label> <input aria-describedby="edit-drupalactiontestslooptest--description" type="checkbox" id="edit-drupalactiontestslooptest" name="Drupal\action\Tests\LoopTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalactiontestslooptest">Actions executing in a potentially infinite loop</label></td><td class="simpletest-test-description"><div class="description">Tests actions executing in a loop, and makes sure they abort properly.</div></td></tr><tr class="action-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\action\Tests\BulkFormTest"><label class="element-invisible" for="edit-drupalactiontestsbulkformtest">Bulk form</label> <input aria-describedby="edit-drupalactiontestsbulkformtest--description" type="checkbox" id="edit-drupalactiontestsbulkformtest" name="Drupal\action\Tests\BulkFormTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalactiontestsbulkformtest">Bulk form</label></td><td class="simpletest-test-description"><div class="description">Tests the views bulk form test.</div></td></tr><tr class="simpletest-group"><td id="aggregator" class="simpletest-select-all"></td><td class="simpletest-group-label"><div class="simpletest-image" id="simpletest-test-group-aggregator"></div><label for="aggregator-select-all" class="simpletest-group-label">Aggregator</label></td><td class="simpletest-group-description">&nbsp;</td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\AddFeedTest"><label class="element-invisible" for="edit-drupalaggregatortestsaddfeedtest">Add feed functionality</label> <input aria-describedby="edit-drupalaggregatortestsaddfeedtest--description" type="checkbox" id="edit-drupalaggregatortestsaddfeedtest" name="Drupal\aggregator\Tests\AddFeedTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsaddfeedtest">Add feed functionality</label></td><td class="simpletest-test-description"><div class="description">Add feed test.</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\AggregatorConfigurationTest"><label class="element-invisible" for="edit-drupalaggregatortestsaggregatorconfigurationtest">Aggregator configuration</label> <input aria-describedby="edit-drupalaggregatortestsaggregatorconfigurationtest--description" type="checkbox" id="edit-drupalaggregatortestsaggregatorconfigurationtest" name="Drupal\aggregator\Tests\AggregatorConfigurationTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsaggregatorconfigurationtest">Aggregator configuration</label></td><td class="simpletest-test-description"><div class="description">Test aggregator settings page.</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\CategorizeFeedTest"><label class="element-invisible" for="edit-drupalaggregatortestscategorizefeedtest">Categorize feed functionality</label> <input aria-describedby="edit-drupalaggregatortestscategorizefeedtest--description" type="checkbox" id="edit-drupalaggregatortestscategorizefeedtest" name="Drupal\aggregator\Tests\CategorizeFeedTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestscategorizefeedtest">Categorize feed functionality</label></td><td class="simpletest-test-description"><div class="description">Categorize feed test.</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\CategorizeFeedItemTest"><label class="element-invisible" for="edit-drupalaggregatortestscategorizefeeditemtest">Categorize feed item functionality</label> <input aria-describedby="edit-drupalaggregatortestscategorizefeeditemtest--description" type="checkbox" id="edit-drupalaggregatortestscategorizefeeditemtest" name="Drupal\aggregator\Tests\CategorizeFeedItemTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestscategorizefeeditemtest">Categorize feed item functionality</label></td><td class="simpletest-test-description"><div class="description">Test feed item categorization.</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\AggregatorRenderingTest"><label class="element-invisible" for="edit-drupalaggregatortestsaggregatorrenderingtest">Checks display of aggregator items</label> <input aria-describedby="edit-drupalaggregatortestsaggregatorrenderingtest--description" type="checkbox" id="edit-drupalaggregatortestsaggregatorrenderingtest" name="Drupal\aggregator\Tests\AggregatorRenderingTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsaggregatorrenderingtest">Checks display of aggregator items</label></td><td class="simpletest-test-description"><div class="description">Checks display of aggregator items on the page.</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\FeedFetcherPluginTest"><label class="element-invisible" for="edit-drupalaggregatortestsfeedfetcherplugintest">Feed fetcher plugins</label> <input aria-describedby="edit-drupalaggregatortestsfeedfetcherplugintest--description" type="checkbox" id="edit-drupalaggregatortestsfeedfetcherplugintest" name="Drupal\aggregator\Tests\FeedFetcherPluginTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsfeedfetcherplugintest">Feed fetcher plugins</label></td><td class="simpletest-test-description"><div class="description">Test the fetcher plugins functionality and discoverability.</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\FeedParserTest"><label class="element-invisible" for="edit-drupalaggregatortestsfeedparsertest">Feed parser functionality</label> <input aria-describedby="edit-drupalaggregatortestsfeedparsertest--description" type="checkbox" id="edit-drupalaggregatortestsfeedparsertest" name="Drupal\aggregator\Tests\FeedParserTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsfeedparsertest">Feed parser functionality</label></td><td class="simpletest-test-description"><div class="description">Test the built-in feed parser with valid feed samples.</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\FeedProcessorPluginTest"><label class="element-invisible" for="edit-drupalaggregatortestsfeedprocessorplugintest">Feed processor plugins</label> <input aria-describedby="edit-drupalaggregatortestsfeedprocessorplugintest--description" type="checkbox" id="edit-drupalaggregatortestsfeedprocessorplugintest" name="Drupal\aggregator\Tests\FeedProcessorPluginTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsfeedprocessorplugintest">Feed processor plugins</label></td><td class="simpletest-test-description"><div class="description">Test the processor plugins functionality and discoverability.</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\ImportOpmlTest"><label class="element-invisible" for="edit-drupalaggregatortestsimportopmltest">Import feeds from OPML functionality</label> <input aria-describedby="edit-drupalaggregatortestsimportopmltest--description" type="checkbox" id="edit-drupalaggregatortestsimportopmltest" name="Drupal\aggregator\Tests\ImportOpmlTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsimportopmltest">Import feeds from OPML functionality</label></td><td class="simpletest-test-description"><div class="description">Test OPML import.</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\FeedLanguageTest"><label class="element-invisible" for="edit-drupalaggregatortestsfeedlanguagetest">Multilingual feeds</label> <input aria-describedby="edit-drupalaggregatortestsfeedlanguagetest--description" type="checkbox" id="edit-drupalaggregatortestsfeedlanguagetest" name="Drupal\aggregator\Tests\FeedLanguageTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsfeedlanguagetest">Multilingual feeds</label></td><td class="simpletest-test-description"><div class="description">Checks creating of feeds in multiple languages</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\RemoveFeedTest"><label class="element-invisible" for="edit-drupalaggregatortestsremovefeedtest">Remove feed functionality</label> <input aria-describedby="edit-drupalaggregatortestsremovefeedtest--description" type="checkbox" id="edit-drupalaggregatortestsremovefeedtest" name="Drupal\aggregator\Tests\RemoveFeedTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsremovefeedtest">Remove feed functionality</label></td><td class="simpletest-test-description"><div class="description">Remove feed test.</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\RemoveFeedItemTest"><label class="element-invisible" for="edit-drupalaggregatortestsremovefeeditemtest">Remove feed item functionality</label> <input aria-describedby="edit-drupalaggregatortestsremovefeeditemtest--description" type="checkbox" id="edit-drupalaggregatortestsremovefeeditemtest" name="Drupal\aggregator\Tests\RemoveFeedItemTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsremovefeeditemtest">Remove feed item functionality</label></td><td class="simpletest-test-description"><div class="description">Remove feed items from a feed.</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\UpdateFeedTest"><label class="element-invisible" for="edit-drupalaggregatortestsupdatefeedtest">Update feed functionality</label> <input aria-describedby="edit-drupalaggregatortestsupdatefeedtest--description" type="checkbox" id="edit-drupalaggregatortestsupdatefeedtest" name="Drupal\aggregator\Tests\UpdateFeedTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsupdatefeedtest">Update feed functionality</label></td><td class="simpletest-test-description"><div class="description">Update feed test.</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\UpdateFeedItemTest"><label class="element-invisible" for="edit-drupalaggregatortestsupdatefeeditemtest">Update feed item functionality</label> <input aria-describedby="edit-drupalaggregatortestsupdatefeeditemtest--description" type="checkbox" id="edit-drupalaggregatortestsupdatefeeditemtest" name="Drupal\aggregator\Tests\UpdateFeedItemTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsupdatefeeditemtest">Update feed item functionality</label></td><td class="simpletest-test-description"><div class="description">Update feed items from a feed.</div></td></tr><tr class="aggregator-test js-hide"><td class="simpletest-test-select"><div class="form-item form-type-checkbox form-item-Drupal\aggregator\Tests\AggregatorCronTest"><label class="element-invisible" for="edit-drupalaggregatortestsaggregatorcrontest">Update on cron functionality</label> <input aria-describedby="edit-drupalaggregatortestsaggregatorcrontest--description" type="checkbox" id="edit-drupalaggregatortestsaggregatorcrontest" name="Drupal\aggregator\Tests\AggregatorCronTest" value="1" class="form-checkbox" /></div></td><td class="simpletest-test-label"><label for="edit-drupalaggregatortestsaggregatorcrontest">Update on cron functionality</label></td><td class="simpletest-test-description"><div class="description">Update feeds on cron.</div></td></tr>
    

    Looking at the diff I can see two things:

    - the original table had class="sticky-enabled responsive-enabled"
    - the original table had zebra striping class="simpletest-group odd", I know this is being killed off but are we doing that during or post Twig conversion?

    thedavidmeister’s picture

    +<div class="simpletest-{{ status }}">{{ summary }}</div>

    This class like it would be better as a standard Attribute() class rather than a string that is half generated in the preprocess and half in the template.

    thedavidmeister’s picture

    Scrap what I said in #32, what is even the point of simpletest-result-summary.html.twig? all it does is renders this tiny little bit of the page:

    <div class="simpletest-pass">42 passes, 0 fails, 0 exceptions, and 5 debug messages</div>

    You can't edit the table rows through the theme system, not even the image that displays. All you can do is change part of the class of the summary, BUT you can't change the "simpletest-pass" class being added to all the table rows about a thousand times on the page - the whole thing is generated in one big page callback and then about 3 divs are run through the theme system - so all you can do in the template is make one class for a small part of the page inconsistent with all the classes on the rest of the page. Completely useless..

    My vote is to get rid of theme('simpletest_result_summary') completely - just turn it into a renderable array in the page callback like everything else on the results page is.

    Then the next question is, if the results page is not run through the theme system why is the test summary page run through the theme system? that's just inconsistent and you can only theme the page that you would have less reason to want to theme out of the two (can touch the list of checkboxes but not the thing with all the green/yellow/red colouring with images?).

    So I'm also going to propose that we choose to either convert the entire page callback of the results page into a Twig template OR move the simpletest-test-table template into a page callback and out of the theme system (making this issue into "remove SimpleTest from the theme system").

    thedavidmeister’s picture

    Assigned: thedavidmeister » Unassigned
    thedavidmeister’s picture

    Issue summary: View changes

    updated manual testing steps

    thedavidmeister’s picture

    FileSize
    2.62 KB
    PASSED: [[SimpleTest]]: [MySQL] 55,659 pass(es). View

    I'm just removing theme_simpletest_result_summary() in this patch to see what the testbots think.

    thedavidmeister’s picture

    Status: Needs work » Needs review
    thedavidmeister’s picture

    FileSize
    10.89 KB
    PASSED: [[SimpleTest]]: [MySQL] 55,366 pass(es). View

    with #27 as well.

    thedavidmeister’s picture

    Status: Needs review » Needs work

    back to needs work because of markup differences from #31

    gnuget’s picture

    Assigned: Unassigned » gnuget
    gnuget’s picture

    FileSize
    11.06 KB
    PASSED: [[SimpleTest]]: [MySQL] 55,644 pass(es). View
    11.26 KB

    Hi!

    I attach a reroll of #37 because of #1978982: Convert simpletest_result_form to a Controller

    now taking in account to #1948374: #type 'table' allow attributes on table cells is fixed and now is possible use attributes (and therefore colspan ) at the cells, instead to work in the diferences on #31 in my next patch i will to try to provide a version using theme_table instead to use a custom twig table.

    gnuget’s picture

    Issue summary: View changes

    Remove sandbox link

    gnuget’s picture

    Status: Needs work » Needs review

    I will switch temporary the status to "needs review" for make sure to my reroll does not broke anything, i will switch back to "needs work" once the testbot check the patch

    gnuget’s picture

    Status: Needs review » Needs work
    gnuget’s picture

    Assigned: gnuget » Unassigned
    Status: Needs work » Needs review
    FileSize
    6.82 KB
    6.09 KB
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1898452-43-twig-simpletest.patch. Unable to apply patch. See the log in the details link for more information. View

    This patch is based in #37 and in #25 where was remove the theme_simpletest_result_summary and the twig table was not added

    c4rl’s picture

    Title: Convert simpletest module to Twig » simpletest.module - Convert theme_ functions to Twig

    Per #1757550-44: [Meta] Convert core theme functions to Twig templates, retitling to indicate this issue applies to theme_ functions, which are lower in priority than PHPTemplate conversion issues.

    intergalactic overlords’s picture

    Issue tags: -Needs manual testing, -Twig

    #43: 1898452-43-twig-simpletest.patch queued for re-testing.

    Status: Needs review » Needs work
    Issue tags: +Needs manual testing, +Twig

    The last submitted patch, 1898452-43-twig-simpletest.patch, failed testing.

    zaphoyd’s picture

    Assigned: Unassigned » zaphoyd
    zaphoyd’s picture

    Status: Needs work » Needs review
    FileSize
    11.29 KB
    FAILED: [[SimpleTest]]: [MySQL] 57,685 pass(es), 0 fail(s), and 12 exception(s). View

    Status: Needs review » Needs work
    Issue tags: -Needs manual testing, -Twig

    The last submitted patch, twig-simpletest.patch, failed testing.

    joelpittet’s picture

    Status: Needs work » Needs review

    #43: 1898452-43-twig-simpletest.patch queued for re-testing.

    ezeedub’s picture

    Issue tags: +Needs manual testing, +Twig

    #48: twig-simpletest.patch queued for re-testing.

    joelpittet’s picture

    Status: Needs review » Needs work
    Issue tags: +Needs reroll

    This one needs another re-roll I think from #40 maybe. Also, kinda disappointed the table got removed, but I'll get over it. Was just hoping that would help themers move that tree structure away from a table in the future and that would give them a better crack at it.

    Anyways, there is lots of new stuff in #48 so @zaphoyd could you explain what you did there? It's got a bunch of new stuff than #40 had if you diff the diffs.

    gnuget’s picture

    #52 Accord with http://drupal.org/node/1898034#comment-7421300 Cottser is agree with you.

    I think to is good idea to the table don't be removed , and if that is the case then the patch in #40 is the path to follow (that patch still contain the table)

    thedavidmeister’s picture

    Well what I was saying in #33:

    Then the next question is, if the results page is not run through the theme system why is the test summary page run through the theme system? that's just inconsistent and you can only theme the page that you would have less reason to want to theme out of the two (can touch the list of checkboxes but not the thing with all the green/yellow/red colouring with images?).

    So I'm also going to propose that we choose to either convert the entire page callback of the results page into a Twig template OR move the simpletest-test-table template into a page callback and out of the theme system (making this issue into "remove SimpleTest from the theme system").

    The table you can theme is not the one with all the images and colours in it...

    zaphoyd’s picture

    Status: Needs work » Needs review
    FileSize
    10.76 KB
    PASSED: [[SimpleTest]]: [MySQL] 55,933 pass(es). View

    In theory this should be a better reroll of #40 than the last one. Hopefully this works!

    Status: Needs review » Needs work
    Issue tags: -Needs manual testing, -Twig, -Needs reroll

    The last submitted patch, 1898452-55-twig-simpletest.patch, failed testing.

    joelpittet’s picture

    Status: Needs work » Needs review
    Issue tags: +Needs manual testing, +Twig, +Needs reroll

    #55: 1898452-55-twig-simpletest.patch queued for re-testing.

    joelpittet’s picture

    I'm a bit torn, this theme doesn't need to be overridden IMO by themers. But it would be nice to have a twig template for internal use and hopefully let the core theme developers play with raw markup instead of the convoluted/confusing mess that is #type=>table.

    I do like the consistency that type table brings, but I think the cost of making the code unintelligible is maybe greater.

    joelpittet’s picture

    @Cottser is going to give a try at the type table conversion and maybe that will remove all simpletest theme functions.

    #1938926: Convert simpletest theme tables to table #type

    Cottser’s picture

    Issue tags: -Needs reroll

    Just tweaking tags.

    thedavidmeister’s picture

    Assigned: zaphoyd » Unassigned
    thedavidmeister’s picture

    Issue tags: +Novice

    Manual testing is a novice task.

    azinoman’s picture

    FileSize
    17.25 KB
    FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/simpletest/simpletest.theme.inc. View

    Rerolled!

    Status: Needs review » Needs work

    The last submitted patch, twig-simpletest-1898452-63.patch, failed testing.

    jenlampton’s picture

    Assigned: Unassigned » jenlampton

    rerolling

    jenlampton’s picture

    Assigned: jenlampton » Unassigned
    Issue tags: -Needs manual testing
    FileSize
    92.13 KB
    93.33 KB
    10.92 KB
    PASSED: [[SimpleTest]]: [MySQL] 58,113 pass(es). View

    rerolled patch. and adding manual testing while I'm at it...

    Before:
    simpletest-before-twig.png

    After:
    simpletest-after-twig.png

    It looks like we're loosing a lot of the classes here. I don't think they were all intentionally dropped. Also, I'm not sure if we're allowed to drop the even/odd until after #1649780: Remove first/last/odd/even classes in favor of CSS3 pseudo selectors

    Cottser’s picture

    There is also #1938926: Convert simpletest theme tables to table #type which last I checked the markup matches up but it's sloooow. See #1938926-18: Convert simpletest theme tables to table #type.

    jenlampton’s picture

    Status: Needs work » Needs review
    FileSize
    1.19 KB
    11.19 KB
    PASSED: [[SimpleTest]]: [MySQL] 58,129 pass(es). View
    90.43 KB

    I confirmed with the core CSS crew that it's okay to drop the even/odd classes, so let's leave those be.

    The attached patch adds the responsiveness back to the table.
    simpletest-after-twig2.png

    jenlampton’s picture

    @Cottser, sorry about the cross-post.
    I like the approach in #1938926: Convert simpletest theme tables to table #type better than having a huge table in a template like we do here, but since it's so slow maybe we should get this in first?

    Cottser’s picture

    Status: Needs review » Needs work
    Issue tags: +Needs profiling, +Needs reroll

    I think this patch might need some profiling anyway. And a reroll.

    joelpittet’s picture

    Status: Needs work » Needs review
    Issue tags: -Novice, -Needs reroll
    FileSize
    11.2 KB
    PASSED: [[SimpleTest]]: [MySQL] 58,458 pass(es). View
    11.48 KB
    PASSED: [[SimpleTest]]: [MySQL] 58,509 pass(es). View
    3.69 KB

    Rerolled and Cleaned up the formatting of the twig table indentation and filled out the docblock.

    joelpittet’s picture

    FileSize
    11.21 KB
    PASSED: [[SimpleTest]]: [MySQL] 59,222 pass(es). View

    Re-rolled again.

    joelpittet’s picture

    FileSize
    3.78 KB
    11.59 KB
    PASSED: [[SimpleTest]]: [MySQL] 57,978 pass(es). View

    Doc cleanup and whitespace cleanup.

    joelpittet’s picture

    Issue summary: View changes

    updated result summary status

    joelpittet’s picture

    robmc’s picture

    Issue summary: View changes
    FileSize
    10.65 KB
    PASSED: [[SimpleTest]]: [MySQL] 63,087 pass(es). View

    Re-roll

    Cottser’s picture

    Issue tags: +Twig conversion
    Cottser’s picture

    joelpittet’s picture

    Issue tags: -Needs profiling +Needs manual testing
    FileSize
    6.66 KB
    FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Failed to run tests: failed during invocation of run-tests.sh. View

    Adding manual testing back to this but removing profiling.

    This has been redone due to the drastic changes from #1938926: Convert simpletest theme tables to table #type

    Hope the changes sit well with all.

    joelpittet’s picture

    FileSize
    6.67 KB
    FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Failed to run tests: failed during invocation of run-tests.sh. View
    809 bytes

    Just realized I missed a colon.

    The last submitted patch, 78: 1898452-twig-simpletest-78.patch, failed testing.

    Status: Needs review » Needs work

    The last submitted patch, 79: 1898452-twig-simpletest-79.patch, failed testing.

    joelpittet’s picture

    Status: Needs work » Needs review
    FileSize
    7.24 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 65,231 pass(es). View
    2.26 KB

    Hmm, should have checked a wider scope before deleting that private function. This abstracts things out a bit more so there isn't duplicate code. Should pass now. *crosses fingers*

    pjezek’s picture

    will profile this issue. Just found out that the *simpletest-test-table.html.twig* is no longer contained in the lastest patch. I assume the *how to test* is just outdated?

    edit:
    - the simpletest-test-table.html.twig looks not to be needed.
    - can't really profile as the request is a post?

    joelpittet’s picture

    This needs an Needs issue summary update re #83.

    sun’s picture

    This closely related issue slightly conflicts with the proposed changes here.

    In addition, I'm not sure whether it is really worth to have a full theme template with preprocess and all for that trivial test summary line... :-/

    I assume we do not have a solution for a sensible middle-ground with Twig yet? Does everything have to be a template...?

    joelpittet’s picture

    @sun Yes, the goal is anything with markup should be in a template. There are exceptions but trying to be as hard on that point as I can be. $output .= '<p>No markup in PHP strings please!.</p>'

    We can render a template with the twig service without using the theme system... don't know if I want to play with that here though because we haven't done it yet.

    joelpittet’s picture

    FileSize
    7.24 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,725 pass(es). View

    Re-rolled.

    joelpittet’s picture

    Title: simpletest.module - Convert theme_ functions to Twig » simpletest.module - Convert theme_simpletest_result_summary functions to Twig
    Issue summary: View changes
    Issue tags: -Needs issue summary update +Novice
    steinmb’s picture

    Assigned: Unassigned » steinmb
    steinmb’s picture

    FileSize
    6.24 KB
    FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,710 pass(es), 33 fail(s), and 42 exception(s). View

    PSR-4 fix

    Status: Needs review » Needs work

    The last submitted patch, 90: 1898452-twig-simpletest-89.patch, failed testing.

    steinmb’s picture

    Status: Needs work » Needs review
    FileSize
    5.93 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,757 pass(es). View

    How about this?

    steinmb’s picture

    Assigned: steinmb » Unassigned
    Status: Needs review » Reviewed & tested by the community
    Issue tags: -Needs manual testing

    before:

    <div class="simpletest-pass">15 passes, 0 fails, 0 exceptions</div>
    

    After:

    <div class="simpletest-pass">
       15 passes, 0 fails, 0 exceptions
    </div>

    Drupal test run

    ---------------
    
    Tests to be run:
      - Drupal\system\Tests\Theme\TwigFilterTest
    
    Test run started:
      Friday, June 6, 2014 - 20:25
    
    Test summary
    ------------
    
    Drupal\system\Tests\Theme\TwigFilterTest                      16 passes
    
    Test run duration: 4 sec
    
    Detailed test results
    ---------------------
    
    
    ---- Drupal\system\Tests\Theme\TwigFilterTest ----  
    
    
    Status    Group      Filename          Line Function
    --------------------------------------------------------------------------------
    Pass      Other      run-tests.sh       695 simpletest_script_run_one_test()
        Enabled modules: twig_theme_test
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        "No author" was successfully rendered.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        "Complete quote after without" was successfully rendered.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        "Only author:" was successfully rendered.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        "No author or date" was successfully rendered.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        "Only date" was successfully rendered.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        "Complete quote again for good measure" was successfully rendered.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        "Marked-up quote" was successfully rendered.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        All attributes printed.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        Class attributes printed in the front, the rest in the back.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        Class attributes printed in the back, the rest in the front.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        Class attributes only printed.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        Boolean attribute printed in the front.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        Without string attribute in the front.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        Attributes printed without id nor class attributes.
    Pass      Other      TwigFilterTest.ph  126 Drupal\system\Tests\Theme\TwigFilte
        All attributes printed again.
    
    Cottser’s picture

    Status: Reviewed & tested by the community » Needs work
    1. +++ b/core/modules/simpletest/simpletest.module
      @@ -72,14 +72,55 @@ function simpletest_js_alter(&$javascript) {
      + * @param  array $summary
      ...
      + * @param  array $summary
      

      Extra space between @param and array in these spots.

    2. +++ b/core/modules/simpletest/simpletest.module
      @@ -72,14 +72,55 @@ function simpletest_js_alter(&$javascript) {
      + * @return array
      + *   returns the pluralized test summary items.
      

      This should probably just be "The pluralized test summary items." https://drupal.org/node/1354#return

    3. +++ b/core/modules/simpletest/simpletest.module
      @@ -72,14 +72,55 @@ function simpletest_js_alter(&$javascript) {
      + *   A concatinated string of the formatted test results.
      

      s/concatinated/concatenated/

    4. +++ b/core/modules/simpletest/templates/simpletest-result-summary.html.twig
      @@ -0,0 +1,21 @@
      + * Available variables:
      + *  label: An optional label to be rendered before the results.
      + *  items: The result pluralized items for each result.
      + *  pass: The number of passed test results.
      + *  fail: The number of failed test results.
      + *  exception: The number of exceptioned test results.
      + *  debug: The number of debuged test results.
      

      These are missing minus signs before each available variable (to make it a list per https://drupal.org/node/1354#lists).

    Cottser’s picture

    And thanks for the recent work on this @steinmb @joelpittet :) it is getting late so I don't mean to be curt.

    amitgoyal’s picture

    Status: Needs work » Needs review
    FileSize
    5.93 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,186 pass(es). View
    2.11 KB

    Please review attached patch for fixes in #94.

    Cottser’s picture

    Status: Needs review » Needs work

    Thanks @amitgoyal!

    +++ b/core/modules/simpletest/simpletest.module
    @@ -72,14 +72,55 @@ function simpletest_js_alter(&$javascript) {
    + *   - debug: The number of debuged test results.
    
    +++ b/core/modules/simpletest/templates/simpletest-result-summary.html.twig
    @@ -0,0 +1,21 @@
    + * - debug: The number of debuged test results.
    

    Spotted one more :)

    s/debuged/debugged/

    amitgoyal’s picture

    Status: Needs work » Needs review
    FileSize
    5.93 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,188 pass(es). View
    615 bytes

    Thanks for your feedback @Cottser. Please review attached patch for fixes in #97.

    Cottser’s picture

    There were two instances to be changed (forgot the /g flag ;)). The string in the Twig template should be updated as well. Thanks!

    amitgoyal’s picture

    FileSize
    5.93 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,232 pass(es). View
    635 bytes

    The string in the Twig template has also been updated as per #99. :)

    Please review.

    Cottser’s picture

    Issue summary: View changes
    Status: Needs review » Reviewed & tested by the community
    FileSize
    5.9 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,838 pass(es). View
    1.96 KB

    The docs still weren't sitting well with me, sorry to jump in but I think this version is easier to understand and scan (see interdiff). This is RTBC from my perspective now, and only docs have changed since the previous RTBC so I'm going for it. Did another manual test myself and it checks out.

    Thanks @amitgoyal for all the recent updates!

    alexpott’s picture

    Status: Reviewed & tested by the community » Fixed

    Committed a6eb26d and pushed to 8.x. Thanks!

    • alexpott committed a6eb26d on 8.x
      Issue #1898452 by joelpittet, amitgoyal, gnuget, steinmb,...

    Status: Fixed » Closed (fixed)

    Automatically closed - issue fixed for 2 weeks with no activity.