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

Update the data in the test cases #18496

Merged
merged 2 commits into from
Jun 16, 2022

Conversation

kceiw
Copy link
Contributor

@kceiw kceiw commented Jun 13, 2022

Description

We use the PowerShell api to parse the cmdlet from the user input. It requires the corresponding module to be installed so that it can import it and use it to extract the parameter names from the user input. But that module (e.g. Az modules) are not installed in build machines. The test cases that need to extract the parameter names will fail. The solution here is to not to use Az cmdlets as the input for the test cases. We use the cmdlets from the built-in modules, which are copied to the test directory. Thus we can maintain minimal set up for the test machines and still be able to test the functionality in the product code.

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
      • {Please put the link here}
    • the markdown help files have been regenerated using the commands listed here

kceiw added 2 commits June 13, 2022 15:16
- Parsing the command parameter requires the module to be loaded. On the machine (e.g. build machine) where the Az modules are not installed, the
  parsing doesn't return the parameters names as expected. Some test cases thus fail. The fix is to remove Az cmdlets from the test cases. Instead, we
  use the cmdlet from the built-in modules in the test cases.
- We used to zip the whole model and read from it. Since we don't need to use Az cmdlet in the test cases, we don't really need to zip the model.
  So we use the json file with crafted built-in PowerShell cmdlet as the data used in the test cases. With that, it's also clear to show the changes
  to the test cases and the data.
Copy link

@mirdaki mirdaki left a comment

Choose a reason for hiding this comment

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

LGTM

@wyunchi-ms wyunchi-ms merged commit 055462f into Azure:wyunchi/predictor-ci Jun 16, 2022
wyunchi-ms pushed a commit that referenced this pull request Jun 22, 2022
* Fix the unit tests.

- Parsing the command parameter requires the module to be loaded. On the machine (e.g. build machine) where the Az modules are not installed, the
  parsing doesn't return the parameters names as expected. Some test cases thus fail. The fix is to remove Az cmdlets from the test cases. Instead, we
  use the cmdlet from the built-in modules in the test cases.

* Replace the way of getting the data.

- We used to zip the whole model and read from it. Since we don't need to use Az cmdlet in the test cases, we don't really need to zip the model.
  So we use the json file with crafted built-in PowerShell cmdlet as the data used in the test cases. With that, it's also clear to show the changes
  to the test cases and the data.

Add Dotnet Core 6 in pipeline.

Add logic for global.json

Add logic for global.json
wyunchi-ms added a commit that referenced this pull request Jun 23, 2022
* Trigger Az.Predictor CI

* Fix the issue when running the unit tests (#18300)

* Fix the issue in unit tests.

- In the module we create a minimum runspace for PowerShell, and use it to parse the command line.
  The mini runspace contains the built-in core modules for PowerShell. We don't need them when we
  package and publish our module because the runtime will have them. That'll reduce the module size.
  So in the module project we exclude `contentfiles` when we referece Microsoft.PowerShell.SDK.
- The issue in the unit tests is that it doesn't find the built-in core module when we create the runspace.
  So we have a reference to Microsoft.PowerShell.SDK in the test project to include `contentfiles` to provide
  the built-in core modules. The unit tests can run successfully.

* Test

* Update the data in the test cases (#18496)

* Fix the unit tests.

- Parsing the command parameter requires the module to be loaded. On the machine (e.g. build machine) where the Az modules are not installed, the
  parsing doesn't return the parameters names as expected. Some test cases thus fail. The fix is to remove Az cmdlets from the test cases. Instead, we
  use the cmdlet from the built-in modules in the test cases.

* Replace the way of getting the data.

- We used to zip the whole model and read from it. Since we don't need to use Az cmdlet in the test cases, we don't really need to zip the model.
  So we use the json file with crafted built-in PowerShell cmdlet as the data used in the test cases. With that, it's also clear to show the changes
  to the test cases and the data.

Add Dotnet Core 6 in pipeline.

Add logic for global.json

Add logic for global.json

* Move AzPredictor from msbuild to task

* Revert empty lines

Co-authored-by: kceiw <mahuang@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants