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

tests: split neon_fixtures.py #4871

Merged
merged 3 commits into from
Aug 3, 2023
Merged

tests: split neon_fixtures.py #4871

merged 3 commits into from
Aug 3, 2023

Conversation

LizardWizzard
Copy link
Contributor

@LizardWizzard LizardWizzard commented Aug 2, 2023

Problem

neon_fixtures.py has grown to unmanageable size. It attracts conflicts.

When adding specific utils under for example fixtures/pageserver things sometimes need to import stuff from neon_fixtures.py which creates circular import. This is usually only needed for type annotations, so typing.TYPE_CHECKING flag can mask the issue. Nevertheless I believe that splitting neon_fixtures.py into smaller parts is a better approach.

Currently the PR contains small things, but I plan to continue and move NeonEnv to its own fixtures.env module. To keep the diff small I think this PR can already be merged to cause less conflicts.

UPD: it looks like currently its not really possible to fully avoid usage of typing.TYPE_CHECKING, because some components directly depend on each other. I e Env -> Cli -> Env cycle. But its still worth it to avoid it in as many places as possible. And decreasing neon_fixture's size still makes sense.

@github-actions
Copy link

github-actions bot commented Aug 2, 2023

1264 tests run: 1211 passed, 0 failed, 53 skipped (full report)


Copy link
Member

@bayandin bayandin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't look thoroughly through the code changes, but splitting neon_fixture totally makes sense!

@LizardWizzard LizardWizzard merged commit 1497a42 into main Aug 3, 2023
@LizardWizzard LizardWizzard deleted the dkr/split-neon-fixtures branch August 3, 2023 14:20
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

Successfully merging this pull request may close these issues.

2 participants