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

chore: Refactor getClusterSharding and add proper unit-tests #17183

Closed
3 tasks
leoluz opened this issue Feb 12, 2024 · 0 comments · Fixed by #17167
Closed
3 tasks

chore: Refactor getClusterSharding and add proper unit-tests #17183

leoluz opened this issue Feb 12, 2024 · 0 comments · Fixed by #17167
Labels
component:core Syncing, diffing, cluster state cache refactoring Refactoring required

Comments

@leoluz
Copy link
Collaborator

leoluz commented Feb 12, 2024

The getClusterSharding function is currently implemented in the commands package. This function has important logic that must be properly unit-tested. The commands packages should be kept as lean as possible without any business logic as there are no tests currently in the majority of those packages.

Suggestion:

  • refactor the getClusterSharding function moving it out of the commands package. The package /controller/sharding seems to be a better place to expose this functionality.
  • The function getClusterSharding needs to have extensive unit test coverage as it drives important logic on how to infer shardings based on different configurations.

Related to #17016 and #17124

Checklist:

  • I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
  • I've included steps to reproduce the bug.
  • I've pasted the output of argocd version.

Describe the bug

To Reproduce

Expected behavior

Screenshots

Version

Paste the output from `argocd version` here.

Logs

Paste any relevant application logs here.
@leoluz leoluz added bug Something isn't working refactoring Refactoring required and removed bug Something isn't working labels Feb 12, 2024
@jgwest jgwest added the component:core Syncing, diffing, cluster state cache label Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:core Syncing, diffing, cluster state cache refactoring Refactoring required
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants