The contract for the entry point function RunTestAsync currently only expected string data types for all its input arguments. This compromises type-safety, especially once this library is consumed by client applications like PAC CLI. One of the recommendations to make this more robust is to modify the contract for RunTestAsync to expect actual types for all of its input arguments wherever possible.
This pull request primarily covers making this change to the contract to support robust type-safety - the RunTestAsync function signature is now RunTestAsync(FileInfo testConfigFile, string environmentId, Guid tenantId, DirectoryInfo outputDirectory, string domain, string queryParams). With this change, there is a change to how optional arguments are handled - now the class library is no longer required to set defaults to optional arguments, and this responsibility is now delegated away to the client - for example, PAC CLI. This way the client is free to choose what the default values are for optional arguments and how they can handle invalid/missing values for all arguments. PAC CLI already has built-in mechanisms to validate mandatory arguments.
This change has brought along several changes, which I have listed below -
Due to the changed function signature - the RunTestAsync now requires testConfigFile, environmentId, tenantId, outputDirectory and domain as its mandatory input arguments, and only queryParams is optional. Now it is completely up to the caller to determine which of these are optional for the user, and the caller is responsible for setting default values for whatever they consider as optional, so that they are properly passed into the function.
As mentioned above, the responsibility of validating optional arguments and setting their defaults has been delegated away to the caller
Unit tests have been updated to reflect the modified function signature, and also to verify the change in responsibility of validating optional arguments.
Checklist
[x] The code change is covered by unit tests. I have added tests that prove my fix is effective or that my feature works
[x] I have performed end-to-end test locally.
[x] New and existing unit tests pass locally with my changes
[x] I have commented my code, particularly in hard-to-understand areas
[x] I have made corresponding changes to the documentation
Description
The contract for the entry point function
RunTestAsync
currently only expectedstring
data types for all its input arguments. This compromises type-safety, especially once this library is consumed by client applications like PAC CLI. One of the recommendations to make this more robust is to modify the contract forRunTestAsync
to expect actual types for all of its input arguments wherever possible.This pull request primarily covers making this change to the contract to support robust type-safety - the
RunTestAsync
function signature is nowRunTestAsync(FileInfo testConfigFile, string environmentId, Guid tenantId, DirectoryInfo outputDirectory, string domain, string queryParams)
. With this change, there is a change to how optional arguments are handled - now the class library is no longer required to set defaults to optional arguments, and this responsibility is now delegated away to the client - for example, PAC CLI. This way the client is free to choose what the default values are for optional arguments and how they can handle invalid/missing values for all arguments. PAC CLI already has built-in mechanisms to validate mandatory arguments.This change has brought along several changes, which I have listed below -
RunTestAsync
now requirestestConfigFile
,environmentId
,tenantId
,outputDirectory
anddomain
as its mandatory input arguments, and onlyqueryParams
is optional. Now it is completely up to the caller to determine which of these are optional for the user, and the caller is responsible for setting default values for whatever they consider as optional, so that they are properly passed into the function.Checklist