Skip to content

Test oci8 & pdo_oci in CI - #8348

Closed
mvorisek wants to merge 1 commit into
php:PHP-8.0from
mvorisek:ci_test_oracle
Closed

mvorisek wants to merge 1 commit into
php:PHP-8.0from
mvorisek:ci_test_oracle

Conversation

@mvorisek

@mvorisek mvorisek commented Apr 12, 2022 •

Copy link
Copy Markdown
Contributor

close #8346

@mvorisek
mvorisek changed the base branch from master to PHP-8.0 April 12, 2022 11:21
Comment thread .github/actions/test-linux/action.yml Resolved
Comment thread .github/actions/setup-oracle/action.yml Resolved
@cmb69 cmb69 linked an issue Apr 12, 2022 that may be closed by this pull request
Comment thread .github/actions/test-linux/action.yml Resolved
Comment thread .github/actions/test-linux/action.yml Resolved
@mvorisek
mvorisek force-pushed the ci_test_oracle branch 4 times, most recently from fc0926c to 7550cd7 Compare April 17, 2022 12:07
@mvorisek
mvorisek marked this pull request as ready for review April 17, 2022 12:07
@mvorisek
mvorisek force-pushed the ci_test_oracle branch 2 times, most recently from 542f6b7 to 378291f Compare April 17, 2022 19:34
@mvorisek

Copy link
Copy Markdown
Contributor Author

@cmb69 any idea how to fix Warning: zend_signal: handler was replaced for signal (2) after startup in Unknown on line 0 issue?

@cjbj

cjbj commented Apr 18, 2022

Copy link
Copy Markdown
Contributor

I expect you have a debug PHP build. That warning goes away with a non-debug build. One thing I don't recall whether I tried was to add DIAG_SIGHANDLER_ENABLED=FALSE to a sqlnet.ora file on the PHP host. I'm sorry I'll be out and won't get a chance to try this for a while, but I'll ask a colleague to test.

@mvorisek

mvorisek commented May 9, 2022

Copy link
Copy Markdown
Contributor Author

@cjbj is it possible to disable the signal override completely from the C code (/wo any special env. var)?

@mvorisek

Copy link
Copy Markdown
Contributor Author

@cjbj any idea how to disable the signal override from the oci8 ext itself?

@cjbj

cjbj commented May 26, 2022

Copy link
Copy Markdown
Contributor

@mvorisek apologies I've been slammed with this Python driver release and haven't been able to play with PHP. As well as the previous suggestion, it may be possible to use DISABLE_INTERRUPT=on in a sqlnet.ora (yes, this is not particularly documented - the Net team tell me it will be). I don't believe there is a way in the connect string to do this, and nothing in an API call that OCI8 could make.

@mvorisek
mvorisek force-pushed the ci_test_oracle branch 2 times, most recently from f2cc01c to 03a5e4d Compare May 26, 2022 19:35
@mvorisek
mvorisek marked this pull request as draft May 26, 2022 20:09
@mvorisek

Copy link
Copy Markdown
Contributor Author

@cjbj thanks, the signal override issues is gone!

these tests are failing:

the first two tests are probably not compatible /w the latest Oracle DB

the last one is probably an issue in php

can you please take a look and advise how to fix them?

@cmb69

cmb69 commented May 27, 2022 •

Copy link
Copy Markdown
Member

That looks like an issue in oci_stmt_param_hook() where PDO::PARAM_NULL is not explicitly handled, contrary to other drivers.

PS: somewhat related issue regarding PDO::PARAM_BOOL: https://bugs.php.net/81586

@mvorisek
mvorisek marked this pull request as draft August 22, 2022 09:15
@mvorisek
mvorisek marked this pull request as ready for review August 22, 2022 09:31
@mvorisek

Copy link
Copy Markdown
Contributor Author

This PR is done, fixed into PHP 8.0 and PHP 8.1 were merged, so tests should pass up too a master. Please merge into PHP 8.0.

@mvorisek
mvorisek deleted the ci_test_oracle branch August 24, 2022 14:50
@mvorisek

Copy link
Copy Markdown
Contributor Author

@cmb69 it seems CI fails randomly - see https://github.com/php/php-src/runs/7998206010?check_suite_focus=true#step:10:108 - do you have any idea how to debug this? Does all table names needs to be unique across all Oracle tests? Or can the tests all be marked somehow to be not run in parallel?

@kocsismate

Copy link
Copy Markdown
Member

According to https://stackoverflow.com/a/4842778/1692726, this may happen because:

Your table is already locked by some query. For example, you may have executed "select for update" and have not yet committed/rollbacked and fired another select query. Do a commit/rollback before executing your query.

So yes, I guess the easiest way to quick fix the issue for now is to add a --CONFLICTS-- section. I'm not sure based on the SO answer if it would work as a long term solution if all these statements were executed in a transaction which were then discarded. But otherwise, using different tables for these tests also seems like a good idea.

@cmb69

cmb69 commented Aug 25, 2022

Copy link
Copy Markdown
Member

If (almost) all tests could be conflicting, instead of a --CONFLICTS-- section for each file, there could be a single file named CONFLICTS in the tests directory.

mvorisek added a commit to mvorisek/php-src that referenced this pull request Aug 26, 2022
kocsismate pushed a commit that referenced this pull request Aug 26, 2022
kocsismate added a commit that referenced this pull request Aug 26, 2022
* PHP-8.0:
  Fix GH-8348 for nightly
kocsismate added a commit that referenced this pull request Aug 26, 2022
* PHP-8.0:
  Fix GH-8348 for nightly
@kocsismate

Copy link
Copy Markdown
Member

@mvorisek The OCI tests are very flaky. Could you please take a look at them? E.g: https://github.com/php/php-src/runs/8290051982?check_suite_focus=true#step:11:112

@mvorisek

Copy link
Copy Markdown
Contributor Author

@kocsismate yes, please see #9430 (comment). It seems the ext/oci8/tests/extauth_*.phpt tests are somehow badly designed. I do not use the tested functions, so any help appreciated.

@cmb69

cmb69 commented Sep 11, 2022

Copy link
Copy Markdown
Member

I'm afraid that none of the core contributors uses oci8 at all, so this turns out to be difficult. Isn't there a possibility to set some connection timeout?

@mvorisek

Copy link
Copy Markdown
Contributor Author

IDK, probably not or the tests will require a big modifications, as the only params are accepted thru the DSN string in oci_connect. Then basic DSN string tests will be useless...

I do not use oci8 either, I have contributed this CI testing and we need robust support for our atk4/data PHP framework, where we want to support all major DB vendors.

I understand the importance to keep the CI green and will try to debug these tests locally /w Oracle docker DB.

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.

oci8 / pdo_oci exts are not tested in CI

5 participants