Skip to content
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

Suggestion: use Firebird embedded engines for tests? #22

Open
fdcastel opened this issue May 13, 2023 · 9 comments
Open

Suggestion: use Firebird embedded engines for tests? #22

fdcastel opened this issue May 13, 2023 · 9 comments

Comments

@fdcastel
Copy link
Contributor

fdcastel commented May 13, 2023

What about to use Firebird embedded to test this project?

I have already collected all needed files here (for Windows). I'm using it to test Firebird dialect of SQLAlchemy against versions 2.5 through 5.0 of Firebird.

Also, the db tests could be generated from scratch, instead of being commited to this repository.

If agreed I'm available to submit PRs.

@fdcastel fdcastel changed the title Suggestion: use Firebird aembedded engines for tests Suggestion: use Firebird embedded engines for tests? May 13, 2023
@pcisar
Copy link
Contributor

pcisar commented May 14, 2023

Well, while I'm not against the idea, I'll insist that it must be an optional alternative, not the default one.

  • Using the environment variable for client library (engine) is acceptable method to use the embedded, as it could be also used to alternate the client library itself (to connect to different server, local or remote). But it has to be optional.
  • FBTEST_HOST couldn't be set to None. I'd use another env. to set it (with localhost as default, and conversion of NONE to None).
  • For user and password, I'd rather use standard ISC_USER and ISC_PASSWORD env. variables.
  • test database specification via env. variable must be optional, not mandatory.

Generally, next month I'd like add support for FB5 to the driver, incl. tests, so I'll also add support for customization as outlined above.

@fdcastel
Copy link
Contributor Author

Suggestions pushed to #23.

@fdcastel
Copy link
Contributor Author

fdcastel commented Jan 6, 2024

@pcisar I'm working again on this one and have already automated both environments for Linux and Windows.

Now I'm only missing the scripts to build fbtest30-src.fbk from scratch.

If you can provide them, I can submit a full PR to build everything from the ground up. Both on Linux and Windows.

@pcisar
Copy link
Contributor

pcisar commented Jan 6, 2024

Well, you can use isql -x (or any other FB admin tool that can extract metadata as SQL) to get such script. Similarly it's possible to convert data stored in test database as SQL INSERTs. The sole problem is that isql is not able to work with ARRAY column values.

@fdcastel
Copy link
Contributor Author

fdcastel commented Jan 7, 2024

I believe this was exactly what I told you, half year ago, in #23.

That's why I'm asking you the source scripts.

@pcisar
Copy link
Contributor

pcisar commented Jan 8, 2024

I know. But the ARRAY and to some extent BLOBs (text are easy since FB supports conversion from varchar or text literals, but binary or other sub types are still a problem) are the reason why test database was manually created and there is no SQL script to create it (it's that way since dawn of FDB time). AFAIK there is still no way around it. Of course, it could be handled by python script using the driver. Kind of weird approach to use driver to prepare test for driver, but they do it anyway to some extent in setup/teardown, so probably why not? But as the current mechanism works for me quite fine for years, I had no reason to spend time to do it.

@fdcastel
Copy link
Contributor Author

fdcastel commented Jan 8, 2024

Understood. But in this case HEX_ENCODE, HEX_DECODE and String Literals in Hex couldn't help?

They are a hell of inefficient (in space terms), but they are available in all versions. Base64 would be much better, but it was added only to Firebird 4.

I'll make some attempts in a few days.

@pcisar
Copy link
Contributor

pcisar commented Jan 9, 2024

Well, those could probably help with binary BLOBs, but not with ARRAYs. I know that almost nobody uses them, but because driver supports them, there should be tests for them. These tests even helped to fix a regression in FB5 :)
As I see it, the only possible solution to have test database created by some fixture/setup would be to give up to fill ARRAYs in SQL script, and change tests for ARRAYs so they would not require values already present in database.
Guess that it would be also worth to rewrite all tests from unittest to pytest, but I don't have time for that.

@fdcastel
Copy link
Contributor Author

fdcastel commented Jan 9, 2024

These tests even helped to fix a regression in FB5 :)

Nice!

Guess that it would be also worth to rewrite all tests from unittest to pytest, but I don't have time for that.

If you are interested in this, I may get some days to work on it.

I have some experience with pytest working on SQLAlchemy Firebird dialect. It's a fantastic tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants