Posted by David_Rothstein on October 20, 2010 at 10:25pm
4 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | filter.module |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | Novice |
Issue Summary
There are a lot of tests in the filter.module and elsewhere throughout Drupal core that have code like this:
<?php
$filtered_html_format = db_query_range('SELECT * FROM {filter_format} WHERE name = :name', 0, 1, array(':name' => 'Filtered HTML'))->fetchObject();
?>That ugly code is no longer necessary anymore due to #934050: Change format into string - instead, we can just use the known, machine-readable name of the format directly, i.e. we could call filter_format_load('filtered_html') in the example above.
Comments
#1
I'm tagging this "Novice", although maybe it's not for extreme novices. But it's a good patch for someone who wants to learn their way around a bunch of core tests.
#2
While somewhat novice, I'd like to ensure some sanity here.
#3
The last submitted patch, drupal.filter-format-tests.2.patch, failed testing.
#4
Stupid copy and paste mistake, sorry. Looks RTBC to me.
#5
The last submitted patch, drupal.filter-format-tests.4.patch, failed testing.
#6
Fixed those test failures:
In FilterAdminTestCase, a new text format is created (or edited) in the internal browser. Afterwards, the test code in the parent site tries to load that format in order to build administration URLs. However, filter_format_load() resp. filter_formats() is statically cached, so the new format does not exist in the parent site.
#7
Removed needless usage of filter_format_load() where possible.
#8
Looks perfectly reasonable to me.
I also checked for places this patch might have missed, and couldn't find any. This seems to catch/fix all cases where the text formats were loaded by human-readable name.
#9
This looks like a good clean-up to me, as long as contrib tests aren't affected by the loss of those two functions. If we receive any reports of breakage, we're going to need to add those back in as dumb wrappers with a "@todo remove in D8" around them.
Committed to HEAD.
#10
Automatically closed -- issue fixed for 2 weeks with no activity.