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.

Sign up for GitHub

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

Jump to bottom

Add unit tests for Coriolis Scheduler RPC Server #257

Merged
merged 1 commit into from
Nov 10, 2023
Merged

Add unit tests for Coriolis Scheduler RPC Server #257

merged 1 commit into from
Nov 10, 2023

Conversation

smiclea
Copy link

@smiclea smiclea commented May 1, 2023

No description provided.

Dany9966
Dany9966 requested changes Oct 24, 2023
View reviewed changes
Copy link
Contributor

@Dany9966 Dany9966 left a comment

Choose a reason for hiding this comment

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

Please add independent test cases for each of the methods (public or not). Please also make sure to do the best assertions that make sense for the particular test case.

coriolis/tests/scheduler/rpc/test_server.py Outdated Show resolved Hide resolved
coriolis/tests/scheduler/rpc/test_server.py Outdated Show resolved Hide resolved
@smiclea
Copy link
Author

smiclea commented Nov 1, 2023

@Dany9966 All comments have been resolved.

@smiclea
Copy link
Author

smiclea commented Nov 7, 2023

@Dany9966 test_get_workers_for_specs function name renamed from test_get_workers_for_specs_refactored

Dany9966
Dany9966 requested changes Nov 7, 2023
View reviewed changes
Copy link
Contributor

@Dany9966 Dany9966 left a comment

Choose a reason for hiding this comment

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

Almost there!

coriolis/tests/scheduler/rpc/test_server.py Show resolved Hide resolved
@smiclea smiclea force-pushed the test-scheduler branch 3 times, most recently from 475d616 to 7c7e194 Compare November 9, 2023 17:16
Dany9966
Dany9966 approved these changes Nov 10, 2023
View reviewed changes
Copy link
Contributor

@Dany9966 Dany9966 left a comment

Choose a reason for hiding this comment

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

LGTM

@Dany9966 Dany9966 merged commit 1d7e51a into cloudbase:master Nov 10, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dany9966 Dany9966 Dany9966 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@smiclea @Dany9966