Skip to content

add test for bug #60994 (Reading a multibyte CLOB caps at 8192 characters) #1566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from

Conversation

ashnazg
Copy link
Contributor

@ashnazg ashnazg commented Oct 12, 2015

Tests to show bug behavior of "Reading a multibyte CLOB caps at 8192 characters", as described in https://bugs.php.net/bug.php?id=60994.

Additional notes are on my question here (http://stackoverflow.com/questions/33026617/pdo-oci-truncates-large-multibyte-clobs) that might help diagnose the bug.

@cjbj: I had to construct this PHPT by hand from my standalone PHP test file, so it may need some polishing to run just right with your test scaffolding (those require targets etc). I'll update PR if you'll highlight any edits that are needed to get it running.

/* $Id$ */
if (!extension_loaded('mbstring') || !extension_loaded('pdo') || !extension_loaded('pdo_oci')) die('skip not loaded');
require dirname(__FILE__).'/../../pdo/tests/pdo_test.inc';
if (!strpos(strtolower(getenv('PDOTEST_DSN_UTF8')), 'charset=al32utf8')) die('skip expected output valid for AL32UTF8 character set');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the different DSN var, PDOTEST_DSN_UTF8... I assumed you'd need a separate DSN for the UTF8 tests.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not using a different DSN for UTF8. DSN string will be changed manually to run UTF8 tests. Hence, please change "PDOTEST_DSN_UTF8" to "PDOTEST_DSN".

@ashnazg
Copy link
Contributor Author

ashnazg commented Oct 12, 2015

I think it's somewhat plausible that the root cause of this bug behavior here could be bug#70700 (#1569).

@krakjoe
Copy link
Member

krakjoe commented Jan 7, 2017

@cjbj please can we get some feedback ?

@cjbj
Copy link
Contributor

cjbj commented Jan 25, 2017

@ashnazg The test looks good.

Before it can be merged

  • the base bug needs to be fixed (no ETA on this yet)
  • the test needs to override the testsuite settings. These need to be added to the test:
$dbh->setAttribute(PDO::ATTR_CASE, PDO::CASE_NATURAL);
$dbh->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, false);

<?php
require 'ext/pdo/tests/pdo_test.inc';
$dbh = PDOTest::factory();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To run in the PDO testsuite the following needs to be added:

$dbh->setAttribute(PDO::ATTR_CASE, PDO::CASE_NATURAL);
$dbh->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, false);

@krakjoe
Copy link
Member

krakjoe commented Feb 22, 2017

@cjbj can we get your approval on this one please ?

@cjbj
Copy link
Contributor

cjbj commented Feb 23, 2017

@krakjoe the base bug would need to be fixed first, so this will have to wait.
(I'm going to sit with @tianfyang next week and review the miscellaneous PRs.)

Copy link

@tianfyang tianfyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We decide to merge this test in first. Given the bug 60994 is not fixed yet, we need to add XFAIL section to indicate this is an expected failure so far.

Thanks so much for your help.

echo 'beg of stream4 is ', $start4, PHP_EOL;
echo 'end of stream4 is ', $ending4, PHP_EOL;


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the following two lines here:
--XFAIL--
Fails due to Bug 60994

We decide to merge this test in first. Given the bug 60994 is not fixed yet, we need to add XFAIL section to indicate this is an expected failure so far.

@ashnazg
Copy link
Contributor Author

ashnazg commented Mar 6, 2017

@tianfyang : requested changes are done.

@php-pulls
Copy link

Comment on behalf of sixd at php.net:

Committed - thank you

@php-pulls php-pulls closed this Jun 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants