-
Notifications
You must be signed in to change notification settings - Fork 20
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
refactor ipam unit tests to maintain data #5163
Conversation
CodSpeed Performance ReportMerging #5163 will not alter performanceComparing Summary
|
|
||
|
||
@pytest.fixture(scope="module") | ||
async def default_branch(local_storage_dir: str, db: InfrahubDatabase) -> Branch: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should function body of these fixtures be factorized into new functions to avoid duplication of backend.tests.helpers
original fixtures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good suggestion, I've made that change
@@ -0,0 +1,58 @@ | |||
from typing import Any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this query is used only for testing purposes, I think it should be located under tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's tricky because the goal is to expose it to other systems during integration tests so from Infrahub point of view it's not just for testing
work from hackathon
adds a new query to reset a database to specific time by hard-deleting any changes after that time
updates some IPAM unit tests to use this new query to speed up some unit tests
would like to follow it up with a mutation that allows doing this, but will require some more work to support updating cached data correctly. made an issue for it here IFC-998