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

Ensure the catalog controller's Reconcile() function is being properly unit tested #198

Open
everettraven opened this issue Oct 13, 2023 · 2 comments

Comments

@everettraven
Copy link
Collaborator

#196 Updated the catalog controller's unit tests which resulted in the removal of using envtest for unit testing purposes. It was called out as part of that PR that we should look into refactoring the Reconcile() function in such a way that makes it easier to test only the logic that runs inside of the Reconcile() function:

A future tech debt item could be to investigate refactoring the Reconcile() function to make it easier to test the logic without using envtest.

@stevekuznetsov
Copy link
Member

Factoring the apply logic into a library (example) and testing that in isolation is likely a better way to ensure correctness. If you do that, the rest of Reconcile() ends up being ~10 lines of code, which will remain static and unchanging forevermore. Visually inspecting the un-recoverable error case may be sufficient then.

@everettraven
Copy link
Collaborator Author

Factoring the apply logic into a library (example) and testing that in isolation is likely a better way to ensure correctness. If you do that, the rest of Reconcile() ends up being ~10 lines of code, which will remain static and unchanging forevermore. Visually inspecting the un-recoverable error case may be sufficient then.

This is along the lines of what I was thinking. As far as I am aware, both rukpak and operator-controller are using a similar Reconcile() function as catalogd so this would likely be useful across multiple OLMv1 components.

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