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

migrate unittest to pytest #438

Merged
merged 8 commits into from
May 1, 2023
Merged

migrate unittest to pytest #438

merged 8 commits into from
May 1, 2023

Conversation

ykim-akamai
Copy link
Contributor

📝 Description

removed unittest library dependencies in all unit tests
removed unittest command from Makefile

✔️ How to Test

What are the steps to reproduce the issue or verify the changes?

make testunit

📷 Preview

If applicable, include a screenshot or code snippet of this change. Otherwise, please remove this section.

Copy link
Contributor

@lgarber-akamai lgarber-akamai left a comment

Choose a reason for hiding this comment

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

Looks great! Just a few small things 🎉

NOTE: It looks like pytest isn't picking up test files that don't contain the test_ prefix. Could files without the prefix have one added?

.github/workflows/unit-tests.yml Show resolved Hide resolved
Copy link
Contributor

@yec-akamai yec-akamai left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -41,7 +40,7 @@ def mock_cli(
# We need this to suppress warnings for operations that don't
# have access to the cli.suppress_warnings attribute.
# e.g. operation defaults
sys.argv.append("--suppress-warnings")
# sys.argv.append("--suppress-warnings")
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can add a bit context on why it needs to be removed here for visibility.

"""
Test if the bash completion renders correctly
"""
# mocker = mocker.patch('linodecli-completion.get_bash_completions', return_value=self.bash_expected)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it seems can be cleaned up.

@ykim-akamai
Copy link
Contributor Author

Looks great! Just a few small things 🎉

NOTE: It looks like pytest isn't picking up test files that don't contain the test_ prefix. Could files without the prefix have one added?

Changed the file naming instead for consistency. E.g. configuration.py -> test_configuration.py

@ykim-akamai ykim-akamai merged commit fcaaf50 into dev May 1, 2023
@zliang-akamai zliang-akamai deleted the migrate-unit-pytest branch August 15, 2023 16:11
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.

5 participants