#35236 closed defect (bug) (fixed)
remove_rewrite_tag()
Reported by: | swissspidy | Owned by: | swissspidy |
---|---|---|---|
Milestone: | 4.5 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Rewrite Rules | Keywords: | has-patch has-unit-tests commit |
Focuses: | Cc: |
Attachments (3)
Change History (11)
#3
in reply to:
↑ 2
;
follow-up:
↓ 4
@
9 years ago
Replying to boonebgorges:
In your tests, you're using
array_pop()
to test the presence of a value in$wp_rewrite
properties. This is dangerous:array_pop()
modifies the original array, which might break later tests. Can you useassertContains()
andassertNotContains()
instead? If it's important to check that it's the last item in the array, maybe you could useend()
?
Ah, thanks for pointing this out! For WP_Rewrite::$rewritereplace
it is indeed important.
For example, when you register custom post types, '(.+)'
gets added to WP_Rewrite::$rewritereplace
multiple times. We only want to remove this once from the array, not all occurrences.
The positions of tags, regexes and query strings inside the arrays need to be in sync because we remove them that way.
Rewrite tags are unique because you can't add one twice (it gets updated instead).
I now switched to using end()
and assertContains()
where needed and added some more tests.
#4
in reply to:
↑ 3
@
9 years ago
Replying to swissspidy:
Ah, thanks for pointing this out! For
WP_Rewrite::$rewritereplace
it is indeed important.
For example, when you register custom post types,
'(.+)'
gets added toWP_Rewrite::$rewritereplace
multiple times. We only want to remove this once from the array, not all occurrences.
The positions of tags, regexes and query strings inside the arrays need to be in sync because we remove them that way.
Rewrite tags are unique because you can't add one twice (it gets updated instead).
I now switched to using
end()
andassertContains()
where needed and added some more tests.
Oh, interesting. Another strategy would be to stash a copy of the array before running remove_rewrite_tag()
, and then compare it to the array after running remove_rewrite_tag()
. But what you have here looks like it should work :)
#5
@
9 years ago
- Keywords commit added
35236.3.diff adds setUp()
/ tearDown()
methods to properly reset the arrays. Also, using array comparison as suggested by @boonebgorges where it makes sense.
In your tests, you're using
array_pop()
to test the presence of a value in$wp_rewrite
properties. This is dangerous:array_pop()
modifies the original array, which might break later tests. Can you useassertContains()
andassertNotContains()
instead? If it's important to check that it's the last item in the array, maybe you could useend()
?