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

Adds allocated resources to vm info #310

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

abhi1693
Copy link
Contributor

Closes: #288

Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

I'm guessing that we don't have good test coverage here, since this addition doesn't trigger any test failures 😅

@abhi1693 are you comfortable enough yet to write some tests here?

@abhi1693
Copy link
Contributor Author

@joechainz Hi Joe, is it possible to help merge this PR? I'm not good with pytest and currently dont have an environment to write integration tests.

@dmurphy18
Copy link
Contributor

@abhi1693 All code requires tests before it should be committed, the lack of tests been written for Salt code created many problems (and this was even with eyes on it from core team members), hence from Salt version 2019.2.0 on, all code required tests be written to exercise the changes.

You should look at this as a learning opportunity for PyTest since it is heavily used now within the industry and it is required for all new code in Salt. There are a number of how-to videos on PyTest with Salt on Twitch, search for Wayne Werner who gave the talks

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.

vmware_vm.info should have allocated resources
3 participants