[SSH] Cast "certificate_validity" as an int before calling ListCredentials

[vthiebaut10]

This has never been a problem before, but a customer suddenly started having issues because ListCredentials was being called with a float instead of an int.

This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

General Guidelines

• Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
• Have you run python scripts/ci/test_index.py -q locally? (pip install wheel==0.30.0 required)

For new extensions:

• My extension description/summary conforms to the Extension Summary Guidelines.

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify src/index.json.

[azure-client-tools-bot-prd]

️✔️Azure CLI Extensions Breaking Change Test

️✔️Non Breaking Changes

[azure-client-tools-bot-prd]

Hi @vthiebaut10,
Please write the description of changes which can be perceived by customers into HISTORY.rst.
If you want to release a new extension version, please update the version in setup.py as well.

[yonzhan]

Thank you for your contribution! We will review the pull request and get back to you soon.

[zhoxing-ms]

@vthiebaut10 Could you add some scenario tests for this case?

[vthiebaut10]

@zhoxing-ms I added a test, but it is not working, and I need some help to make it work.
In the function I modified the ListCredentials method is called with the following syntax

azure-cli-extensions/src/ssh/azext_ssh/connectivity_utils.py

Line 185 in f311e02

return ListCredential(cli_ctx=cmd.cli_ctx)(command_args=list_cred_args)

I believe that this is implicitly calling the ListCredentials.__call__ method with the command_args parameter. I want to test that, even if a non-integer value is passed in as the certificate_validity_in_seconds variable, that the ListCredentials.__call__ method is being called with an integer value.

I added this test:

@mock.patch('azext_ssh.aaz.latest.hybrid_connectivity.endpoint.ListCredential')
    def test_list_credentials_with_float_validity(self, mock_list_credential):
        cmd = mock.Mock()
        cmd.cli_ctx = mock.Mock()

        mock_list_credential_instance = mock.Mock()
        mock_list_credential_instance.__call__ = mock.Mock()
        mock_list_credential.return_value = mock_list_credential_instance

        connectivity_utils._list_credentials(cmd, "resource_uri", 100.67)
        mock_list_credential_instance.__call__.assert_called_once_with(command_args={'endpoint_name': 'default', 'resource_uri': 'resource_uri', 'expiresin': 100, 'service_name': 'SSH'})

But this doesn't work because of the syntax of how ListCredentials.__call__ is called implicitly by the code. The test passes if I change the code to call ListCredentials.__call__ explicitly, but I'd like to avoid making changes to the code.

Is there a way that I can modify this test to be able to check that ListCredentials.__call__ is being called with the correct arguments, even if it is being called implicitly?

[zhoxing-ms]

@vthiebaut10 Sorry, I don't have any prior experience with this issue, and I currently don't have enough time to delve into the details of Python mock test.
In order to quickly give you some potentially helpful clues, I asked GPT-4 about this question
The answer I received is this:

The assert_called_once_with method in Python's unittest.mock library is used to assert that a mock was called exactly once and that it was called with a specific set of arguments. 
It does not check for "implicitly" calls, but rather it checks for the explicit call that was made to the mock.

Please note that the answers from LLM are for reference only

[vthiebaut10]

@zhoxing-ms I spent a good amount of time looking for a solution for this, but I didn't find it. Would it be ok to merge this PR without a test case? the change is very minimal and it's very unlikely it would break anything. If that is ok I will remove this test and add it once I find a way to test it.

[zhoxing-ms]

@vthiebaut10 Okay, if you have already verified its effectiveness through other ways, then you can skip adding tests for now to avoid blocking this PR.

[vthiebaut10]

@zhoxing-ms Removed the test. Tested the change in our team's end-to-end test pipeline. If this is good enough, feel free to merge and release this fix.

[azclibot]

[Release] Update index.json for extension [ ssh ] : https://dev.azure.com/azclitools/internal/_build/results?buildId=137326&view=results