Add support for chocolatey source priority.

[Linuturk]

What does this PR do?

This PR adds the priority switch to the Chocolatey State and Execution Modules. https://docs.chocolatey.org/en-us/choco/commands/sources#options-and-switches

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

• Docs
• Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

[Linuturk]

I examined the existing test cases and I'm not seeing any existing testing for the various parameters currently supported.

The failures in the checks seem to be related to a missing directory in the junit step, but I'm not familiar enough with the test suite to know why or how I could fix that.

[garethgreenaway]

@Linuturk Thanks for the PR. One question, usually when adding new arguments into a function we like to see those added to the end to maintain compatibility, was there a reason you added this new one to the middle? Thanks!

[Linuturk]

@garethgreenaway I wasn't aware of that convention, and I placed the priority option next to the source as it seemed the most related to the option in my mind. I'm not married to that location if that's blocking adding the feature.

[garethgreenaway]

@Linuturk Yes please. As I said we don't wait to break functionality for anyone whose relying on the argument order currently. Thanks!

[Linuturk]

Hey @garethgreenaway I've made the requested changes. Please let me know if there's anything else!

[garethgreenaway]

@Linuturk Looks good. Would you be able to write some tests, or update existing ones, to support the changes? Let us know if you require assistance.

[Linuturk]

Hello @garethgreenaway I had trouble finding any existing tests for this code. Are there existing tests you could point me to?

[garethgreenaway]

@Linuturk there are these tests under the tests directory in the code base:

./pytests/unit/modules/test_chocolatey.py
./integration/modules/test_chocolatey.py
./integration/states/test_chocolatey.py

[garethgreenaway]

@Linuturk Looking at the files I sent I see there are no existing tests for the functions that you've extended.

[garethgreenaway]

@Linuturk Here is a test that you can add to pytests/unit/modules/test_chocolatey.py that will test out the changes you made.

def test_add_source(choco_path):
    """
    Test add_source when remote is False
    """
    cmd_run_all_mock = MagicMock(return_value={"retcode": 0, "stdout": "data"})
    cmd_run_which_mock = MagicMock(return_value=choco_path)
    with patch.dict(chocolatey.__salt__, {"cmd.which": cmd_run_which_mock,
                                          "cmd.run_all": cmd_run_all_mock}):
        expected_call = [
            choco_path,
            "sources",
            "add",
            "--name",
            "source_name",
            "--source",
            "source_location",
        ]

        result = chocolatey.add_source("source_name", "source_location")
        cmd_run_all_mock.assert_called_with(expected_call, python_shell=False)

        expected_call = [
            choco_path,
            "sources",
            "add",
            "--name",
            "source_name",
            "--source",
            "source_location",
            "--priority",
            "priority"
        ]

        result = chocolatey.add_source("source_name", "source_location", priority="priority")
        cmd_run_all_mock.assert_called_with(expected_call, python_shell=False)

[garethgreenaway]

@Linuturk And one more so we have a test for the source_present function in the state.

import logging

import pytest

import salt.config
import salt.modules.chocolatey as chocolatey_mod
import salt.states.chocolatey as chocolatey
from tests.support.mock import MagicMock, patch

log = logging.getLogger(__name__)


@pytest.fixture(scope="module")
def choco_path():
    return "C:\\path\\to\\chocolatey.exe"


@pytest.fixture
def configure_loader_modules():
    opts = salt.config.DEFAULT_MINION_OPTS.copy()
    return {
        chocolatey: {
            "__opts__": opts,
            "__salt__": {},
            "__context__": {},
        },
        chocolatey_mod: {
            "__opts__": opts,
            "__context__": {},
        },
    }


@pytest.fixture(scope="module")
def pkgs():
    return {
        "pkga": {"old": "1.0.1", "new": "2.0.1"},
        "pkgb": {"old": "1.0.2", "new": "2.0.2"},
        "pkgc": {"old": "1.0.3", "new": "2.0.3"},
    }


@pytest.fixture(scope="module")
def list_sources():
    _ret = {
        "chocolatey": {
            "URL": "https://community.chocolatey.org/api/v2/",
            "Disabled": False,
            "User": "user",
        },
        "community": {
            "URL": "https://community.chocolatey.org/api/v2/",
            "Disabled": False,
            "User": "user",
        },
    }
    return _ret


def test_source_present(list_sources):
    """
    Test chocolatey.source_present with simulated changes
    """

    before_list_sources = {
        "chocolatey": {
            "URL": "https://community.chocolatey.org/api/v2/",
            "Disabled": False,
            "User": "user",
        }
    }

    list_sources_sideeffect = MagicMock(side_effect=[before_list_sources, list_sources])

    with patch.dict(
        chocolatey.__salt__,
        {
            "chocolatey.list_sources": list_sources_sideeffect,
        },
    ):

        # Run state with test=true
        stdout_ret = (
            "Added community - https://community.chocolatey.org/api/v2/ (Priority 5)"
        )
        cmd_run_all_mock = MagicMock(return_value={"retcode": 0, "stdout": stdout_ret})
        cmd_run_which_mock = MagicMock(return_value=choco_path)
        with patch.dict(
            chocolatey.__salt__,
            {
                "chocolatey.add_source": chocolatey_mod.add_source,
            },
        ), patch.dict(
            chocolatey_mod.__salt__,
            {
                "cmd.which": cmd_run_which_mock,
                "cmd.run_all": cmd_run_all_mock,
            },
        ):
            ret = chocolatey.source_present(
                "community",
                source_location="https://community.chocolatey.org/api/v2/",
                username="username",
                password="password",
                priority="5",
            )
            assert ret["result"] is True
            assert ret["name"] == "community"
            assert ret["comment"] == "Source community added successfully"
            assert ret["changes"] == {
                "community": {
                    "old": "",
                    "new": {
                        "URL": "https://community.chocolatey.org/api/v2/",
                        "Disabled": False,
                        "User": "user",
                    },
                }
            }

One thing that might be helpful in a future change (or this one if you want) would be to update the list function in the chocolatey module to include the priority in the results it is returning.

[Linuturk]

Thank you for allt his help with the tests. Where would I place that second section of code you provided?

[garethgreenaway]

@Linuturk that would be a new file that should go under ./pytests/unit/states/ in a file called test_chocolatey.py