Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add driver unit tests #3

Merged
merged 2 commits into from
Apr 12, 2024
Merged

Add driver unit tests #3

merged 2 commits into from
Apr 12, 2024

Conversation

stpierre
Copy link
Contributor

@stpierre stpierre commented Apr 4, 2024

No description provided.

@stpierre stpierre force-pushed the stpierre/driver-unit-tests branch from c3e26fe to 2290c67 Compare April 9, 2024 19:09
@stpierre stpierre changed the title Driver unit tests, but gosh they're ugly Add driver unit tests Apr 9, 2024
@stpierre stpierre force-pushed the stpierre/driver-unit-tests branch 2 times, most recently from 9ca6331 to 8b72551 Compare April 9, 2024 19:21
@stpierre stpierre marked this pull request as ready for review April 9, 2024 21:09
@stpierre stpierre requested a review from a team April 9, 2024 21:09
Comment on lines 44 to 42
def __call__(
self,
mock_obj: Any, # the k8s model objects have no common superclass
state: str | None = None,
state_contains: str | None = None,
) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have this be a __call__ function rather than like assert_consistent_state() or something like that?

I tried to eliminate as much code duplication as possible without
being excessively magical. There's still a healthy amount of
duplication, but I'm not sure there's much to be done about it at this
point without getting deep into metaprogramming.
@stpierre stpierre force-pushed the stpierre/driver-unit-tests branch from 5fd799f to b81ed63 Compare April 10, 2024 17:44
@stpierre stpierre requested a review from isaac-ped April 12, 2024 12:05
Copy link
Contributor

@isaac-ped isaac-ped left a comment

Choose a reason for hiding this comment

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

Beautiful

# (https://github.com/kubernetes-client/python/issues/225), and I
# couldn't get any of the third-party stub packages (e.g.,
# https://pypi.org/project/kubernetes-stubs-elephant-fork/) to work.
ignore = ["kwait/drivers.py", "test/test_drivers.py"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Annoying :(

Copy link
Contributor

Choose a reason for hiding this comment

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

You might consider this solution: microsoft/pyright#945
So that we can ignore the imports from that module without ignoring typing in those files altogether

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to merge this as-is and try to work on the typing stuff in a different PR to make it clearer what's happening. This looks promising though....

@stpierre stpierre merged commit 8e86966 into main Apr 12, 2024
1 check passed
@stpierre stpierre deleted the stpierre/driver-unit-tests branch April 12, 2024 17:24
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants