-
Notifications
You must be signed in to change notification settings - Fork 7.9k
ext/dba/tests/dba_tcadb.phpt: fix expected output #11648
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
Conversation
This now fail on CI and on my own machine too. The problem with DBA is that... drivers and databases can implement stuff a bit how they want AFAIK and do not support the same feature set necessarily |
Hm. Should I just use a wildcard on that line then? |
f1b0d70
to
49240fd
Compare
Do you hit a different output on your machine? |
Yeah, I get the extra I wonder what the difference is. I've got tokyocabinet-1.4.48. |
I don't think it's the version of the library as I also have |
Neither fc38 nor Gentoo have any weird TokyoCabinet patches, so who knows. In any case, reading https://www.php.net/manual/en/function.dba-open.php, it says "read during write is not allowed" and that the no-lock mode should be used only if "you are absolutely sure that you do not require database locking." If we take that as our source of truth, then to me it sounds like we shouldn't expect anything to happen reliably if the user lies to PHP and tells it not to use a lock while attempting reads in the middle of a write. |
The test is just a smoke screen tests that is generic to all drivers that I wrote so that I could refactor ext/dba, moreover I'm not sure relying on the written docs is that wisest thing to do as the canonical source of truth is the implementation. But having a wildcard for the test makes sense I suppose, even tho I don't understand why you get different behaviour. |
Several of our DBA tests (based on setup/setup_dba_tests.inc) have a "no lock" version whose expected output is, SAME OUTPUT AS PREVIOUS RUN (modulo read during write due to no lock) This tokyocabinet test, however, is missing the "modulo" bit, because it is not output when tokyocabinet is built with pthreads support, as is the case on at least Fedora and the GitHub CI. To additionally support systems where tokyocabinet is built WITHOUT pthreads support, this commit adds a wildcard at the end of the expected output to catch the " (modulo..." string.
49240fd
to
1707937
Compare
Found it! It changes when you build tokyocabinet with(out) |
Maybe it makes more sense to SKIPIF if the extension is enabled? |
I suppose it would be even better to have two tests, one for threaded TokyoCabinet and one for threadless. But how would we implement the SKIPIF? The only way that I know of at the moment to detect a threadless TokyoCabinet is to run your test :) |
Yeah... I look into a bit of Tokyo Cabinet and learned a couple of things, it got superseded twice by other libraries from the same author. And that it doesn't seem to expose via its public API if it was compiled with pthreads. And I can't seem to find how one would get access to the information stored in So yeah that is annoying. |
…1648) Several of our DBA tests (based on setup/setup_dba_tests.inc) have a "no lock" version whose expected output is: SAME OUTPUT AS PREVIOUS RUN (modulo read during write due to no lock) This tokyocabinet test, however, is missing the "modulo" bit, because it is not output when tokyocabinet is built with pthreads support, as is the case on at least Fedora and the GitHub CI. To additionally support systems where tokyocabinet is built WITHOUT pthreads support, this commit adds a wildcard at the end of the expected output to catch the " (modulo..." string.
Several of our DBA tests (based on setup/setup_dba_tests.inc) have a "no lock" version whose expected output is,
SAME OUTPUT AS PREVIOUS RUN (modulo read during write due to no lock)
This particular tokyocabinet test, however, is missing the "modulo" bit, causing the lock-free test to fail. Comparing with, for example, ext/dba/tests/dba_{flat,ini}file.phpt, it looks like the right thing to do is simply to add it.