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

Add more unit tests for coriolisclient.cli.* modules #84

Merged
merged 7 commits into from
May 24, 2024

Conversation

Cristi1324
Copy link
Contributor

This PR adds unit tests for coriolisclient.cli.migrations...services modules.

@Cristi1324 Cristi1324 changed the title Add more unit tests for coriolisclient.cli.* modules Add more unit tests for coriolisclient.cli.* modules May 13, 2024
@Cristi1324 Cristi1324 force-pushed the add_cli_unit_tests_2 branch 3 times, most recently from 382084a to 706eff0 Compare May 14, 2024 12:28
"types": [1, 2, -1]
}

result = self.provider._get_formatted_data(obj)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also please add an assertLogs?

self.assertEqual(
(
mock.sentinel.type,
'"mock_schema"'
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think I like this result for json.dumps. Could you please add a more complex example? Including objects and lists?

exceptions.CoriolisException,
replica_schedules._parse_expiration_date,
value
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a test case for _add_schedule_group where you at least check that arg group 'Scheduleis added to the passedparser`.

(
mock.sentinel.id,
('mock_instance3%(ls)smock_instance1%(ls)smock_instance2'
% {"ls": os.linesep}),
Copy link
Contributor

Choose a reason for hiding this comment

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

This could fail if run on Windows. Please use '\n' instead

Suggested change
% {"ls": os.linesep}),
% {"ls": "\n"}),

@Dany9966 Dany9966 merged commit 329fde6 into cloudbase:master May 24, 2024
4 checks passed
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