Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Here is a first test suite for unicode.inc. It demonstrates several cool breakages.
Comment | File | Size | Author |
---|---|---|---|
#15 | unicode-352359-d6-15.patch | 3.07 KB | Albert Volkman |
#10 | 352359-followup-D7.patch | 2.7 KB | Dave Reid |
#2 | unicode-inc-test.patch | 10.08 KB | Damien Tournoud |
unicode-inc-test.patch | 4.55 KB | Damien Tournoud |
Comments
Comment #1
lilou CreditAttribution: lilou commentedSee also #276453: Tests needed: unicode.inc.
Comment #2
Damien Tournoud CreditAttribution: Damien Tournoud commentedMerged #276453: Tests needed: unicode.inc tests into this one.
The tests revealed three issues in drupal_strpos(), two of them were already documented:
Comment #3
grendzy CreditAttribution: grendzy commentedalso related:
#212130: decode_entities() doesn't support all entities
Comment #4
Dries CreditAttribution: Dries commentedThis looks like a good patch. I was going to commit it, but then decided to wait for a reply to #3.
Comment #5
Damien Tournoud CreditAttribution: Damien Tournoud commented@Dries: #212130: decode_entities() doesn't support all entities is a valid issue. The entity decode test (mostly written by Charlie in #276453: Tests needed: unicode.inc), will be easily extended by the additional entities we should manage.
Comment #6
Damien Tournoud CreditAttribution: Damien Tournoud commented#212130: decode_entities() doesn't support all entities now has a proper patch (and test extension), that depends on this one.
To clarify:
- This patch introduce a test for decode_entities (written by Charlie in #276453: Tests needed: unicode.inc). The test doesn't cover any of the missing entities so it pass correctly
- The patch in #212130: decode_entities() doesn't support all entities extends the tests of that patch and adds the missing entities (along with very significant performance and memory usage enhancements).
Comment #7
grendzy CreditAttribution: grendzy commentedI tested #2, and it looks to be working well. Thanks!
Comment #8
Dries CreditAttribution: Dries commentedI committed this patch to CVS HEAD. We'll want to backport part of this patch to Drupal 6 so updating the version string.
Comment #9
Damien Tournoud CreditAttribution: Damien Tournoud commentedComment #10
Dave ReidQuick code style followup for 7.x, then we can mark to for 6.x.
Comment #11
Damien Tournoud CreditAttribution: Damien Tournoud commentedNo, please don't. This non-standard indenting is the only way to make sense out of the test case when reading it.
Comment #12
Dave ReidYeah, I just came back to comment that I completely missed what was going on with the indenting. It would make sense to have it that way when writing the test, but I'm not sure it's really needed though just for reading the test.
Comment #13
Damien Tournoud CreditAttribution: Damien Tournoud commented@Dave: I think it makes sense to keep it that way, but I won't battle over it. Feel free to re-bump to D7 if you really don't like it.
Comment #14
grendzy CreditAttribution: grendzy commentedBTW modules/simpletest/tests/unicode.test will work unmodified with D6 SimpleTest 2.5. The drupal_substr() bug still exists in D6, though.
Comment #15
Albert Volkman CreditAttribution: Albert Volkman commentedD6 backport.
Comment #16
Albert Volkman CreditAttribution: Albert Volkman commentedComment #18
izmeez CreditAttribution: izmeez commentedI see there is a Pull Request in the github repo (https://github.com/d6lts/drupal/pull/29).
Since this might be of interest or use to others moving the issue to the D6LTS issue queue.