Here is a first test suite for unicode.inc. It demonstrates several cool breakages.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lilou’s picture

Component: base system » tests
Status: Active » Needs review
Damien Tournoud’s picture

FileSize
10.08 KB

Merged #276453: Tests needed: unicode.inc tests into this one.

The tests revealed three issues in drupal_strpos(), two of them were already documented:

grendzy’s picture

Dries’s picture

This looks like a good patch. I was going to commit it, but then decided to wait for a reply to #3.

Damien Tournoud’s picture

@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.

Damien Tournoud’s picture

#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).

grendzy’s picture

Status: Needs review » Reviewed & tested by the community

I tested #2, and it looks to be working well. Thanks!

Dries’s picture

Version: 7.x-dev » 6.x-dev

I committed this patch to CVS HEAD. We'll want to backport part of this patch to Drupal 6 so updating the version string.

Damien Tournoud’s picture

Status: Reviewed & tested by the community » Patch (to be ported)
Dave Reid’s picture

Version: 6.x-dev » 7.x-dev
Status: Patch (to be ported) » Needs review
FileSize
2.7 KB

Quick code style followup for 7.x, then we can mark to for 6.x.

Damien Tournoud’s picture

Version: 7.x-dev » 6.x-dev
Status: Needs review » Patch (to be ported)

Quick code style followup for 7.x, then we can mark to for 6.x.

No, please don't. This non-standard indenting is the only way to make sense out of the test case when reading it.

Dave Reid’s picture

Yeah, 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.

Damien Tournoud’s picture

@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.

grendzy’s picture

BTW modules/simpletest/tests/unicode.test will work unmodified with D6 SimpleTest 2.5. The drupal_substr() bug still exists in D6, though.

Albert Volkman’s picture

Component: tests » ajax system
FileSize
3.07 KB

D6 backport.

Albert Volkman’s picture

Status: Patch (to be ported) » Needs review

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.

izmeez’s picture

Title: Unit test unicode.inc » [core] Fixes for drupal_strpos()
Project: Drupal core » Drupal 6 Long Term Support
Version: 6.x-dev »
Component: ajax system » Code
Issue summary: View changes
Status: Closed (outdated) » Needs review

I 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.