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

fix: Pytest modifies and updates search_library sqlite file #646

Closed
wants to merge 1 commit into from

Conversation

Computerdores
Copy link
Collaborator

Copies the search library to a temporary directory before opening it, therefore avoiding changes to the .sqlite file in the repo.
Close #644.

@Computerdores Computerdores added Type: Bug Something isn't working as intended Status: Review Needed A review of this is needed Type: Tests Tests or testing related Priority: High An important issue requiring attention labels Dec 14, 2024
@Tishj
Copy link

Tishj commented Dec 15, 2024

Not very familiar with the tests or the importance of search_library, so take this with a grain of salt;
Is it not a better practice to open search_library.sqlite in read-only mode? explicitly making a temporary copy for the tests that do require making modifications?

That way it should prevent bugs where methods unexpectedly make changes to the search_library, when they are perceived to be read-only instructions

And also prevent tests making changes to the search_library that cause the behavior of other tests to change, depending on the order the tests are ran in? (not sure how your current solution deals with lifetimes like this, is the copied search_library cleaned up at the end of the test?)

@CyanVoxel
Copy link
Member

Not very familiar with the tests or the importance of search_library, so take this with a grain of salt; Is it not a better practice to open search_library.sqlite in read-only mode? explicitly making a temporary copy for the tests that do require making modifications?

That way it should prevent bugs where methods unexpectedly make changes to the search_library, when they are perceived to be read-only instructions

And also prevent tests making changes to the search_library that cause the behavior of other tests to change, depending on the order the tests are ran in? (not sure how your current solution deals with lifetimes like this, is the copied search_library cleaned up at the end of the test?)

This was something I was thinking of as well when initially trying to fix #644. I'm not sure if there's a different way, but trying to add the readonly options create_engine([...].execution_options(postgresql_readonly=True)) or create_engine([...], execution_options={"postgresql_readonly": True}) didn't seem to make any difference and still let the databases be written to. Admittedly I'm learning this ORM as we go, so there's a good chance I'm just not using the proper methods to open the database in readonly mode here.

@Computerdores
Copy link
Collaborator Author

Closed in favor of #648

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High An important issue requiring attention Status: Review Needed A review of this is needed Type: Bug Something isn't working as intended Type: Tests Tests or testing related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Pytest modifies and updates search_library sqlite file
3 participants