Posted by alonpeer on September 5, 2009 at 10:00am
8 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | taxonomy.module |
| Category: | task |
| Priority: | normal |
| Assigned: | rocket_nova |
| Status: | closed (fixed) |
| Issue tags: | needs backport to D7, Novice |
Issue Summary
Added a test, asserting the link is there.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| taxonomy-test-breadcrumbs.patch | 1.5 KB | Idle | Passed: 14734 passes, 0 fails, 0 exceptions | View details |
Comments
#1
+++ modules/taxonomy/taxonomy.test 5 Sep 2009 09:59:31 -0000@@ -438,6 +438,23 @@ class TaxonomyTermTestCase extends Taxon
+ //$this->assertText(l($term2->name, 'taxonomy/term/' . $term2->tid), t('Parent term link is displayed when viewing the node.'));
+ //$this->assertLink($term2->name, t('Parent term link is displayed when viewing the node.'));
Please don't leave commented-out lines of code lying around.
I'm on crack. Are you, too?
#2
Re-roll patch from #1.
#3
Thanks for your work on this patch. One minor correction for the doxygen:
+++ b/modules/taxonomy/taxonomy.testundefined@@ -826,6 +826,21 @@ class TaxonomyTermTestCase extends TaxonomyWebTestCase {
+ * Test that the parent term is linked to from the child term page.
This should begin "Tests" rather than "Test". Reference: http://drupal.org/node/1354#functions
Also note that the Drupal 8.x patch will need to be rerolled, because the core directory structure for Drupal 8 has now changed. (For more information, see #22336: Move all core Drupal files under a /core folder to improve usability and upgrades). When the patch has been rerolled, please set the issue back to "Needs Review."
Tagging as novice for the task of rerolling the Drupal 8.x patch with the above doxygen fix.
#4
@xjm Got it. Thanks for the feedback. Here's the re-rolled patch.
Also, I noticed that there are plenty of other docblocks that aren't of the right tense (e.g. says "Test" instead of "Tests"). Should I create a new issue for all of those in this file?
#5
Thanks @rocket_nova! Regarding the other docblocks with the wrong tense, they will be cleaned up in #1326644: Clean up API docs for taxonomy module.
Edit: I was about to RTBC, but then I noticed the wordings are a bit wonky in the comments:
+++ b/core/modules/taxonomy/taxonomy.testundefined@@ -826,6 +826,21 @@ class TaxonomyTermTestCase extends TaxonomyWebTestCase {
+ * Tests that the parent term is linked to from the child term page.
Here the juxtaposition of "to" and "from" is confusing. I'd change this to:
+++ b/core/modules/taxonomy/taxonomy.testundefined@@ -826,6 +826,21 @@ class TaxonomyTermTestCase extends TaxonomyWebTestCase {
+ // Create two taxonomy terms and set term2 the parent of term1.
Looks like the word "as" is missing. I think it should be:
+++ b/core/modules/taxonomy/taxonomy.testundefined@@ -826,6 +826,21 @@ class TaxonomyTermTestCase extends TaxonomyWebTestCase {
+ // Check for breadcrumb links to parent term.
And I'd just rewrite this as:
Let me know if you're up to reroll again with these changes. Thanks!
#6
Thanks @xjm! Here's the re-rolled patch with your suggestions.
#7
#6: taxonomy-test-569076-6.patch queued for re-testing.
#8
The last submitted patch, taxonomy-test-569076-6.patch, failed testing.
#9
Hm, looks like we need a reroll here. Once it's rerolled I think this is RTBC. Sorry for letting this one slip by me earlier. :)
#10
pickaxe is BUSTED!
git log -Staxonomy_get_term_by_name
Patch applied fine for me after applying to that commit. Committed and rebased for the attached.
#11
#12
Thanks wamilton!
#13
This patch looks good.
Was it a follow-up from another issue?
#14
Nice find. Committed and pushed to 8.x and 7.x. Thanks!
#15
Automatically closed -- issue fixed for 2 weeks with no activity.