Skip to content

Creates PowerShell Samples / Tests #449

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

Merged
merged 44 commits into from
Nov 8, 2022
Merged

Creates PowerShell Samples / Tests #449

merged 44 commits into from
Nov 8, 2022

Conversation

VasuBhog
Copy link
Contributor

@VasuBhog VasuBhog commented Nov 3, 2022

Created the samples and tests following C# / Javascript samples.

The PowerShell integration issues will be tracked here:
#448

Had to update the local.settings.json in order to pass the ad hoc build.

Other known issues found while developing:
If a test fails, the function host won't start as the port is unavailable and therefore needs to kill the port to continue some of the tests. (Plan to create a fix in another PR).
PowerShell worker issue: #443

@VasuBhog
Copy link
Contributor Author

VasuBhog commented Nov 4, 2022

AddProductWithIdentity seems to be failing at random as it passes in some of the ad-hoc builds but then a different AddProductWithIdentity test fails in PR Validation.

I will be marking all of them as unstable for now, and track here

Copy link
Contributor

@Charles-Gagnon Charles-Gagnon left a comment

Choose a reason for hiding this comment

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

  1. Make sure you merge in latest from main to remove the serialization changes
  2. If you haven't been able to verify that it's using the locally built extension DLL during tests (both locally and in the pipeline) then what would probably be best is to remove Powershell from the list of supported languages and then follow up with a PR that actually enables the tests (along with any changes necessary to run using the locally built DLL). This will make that simpler to review and get this in so that others can at least see the various samples and use them while giving you time to figure out the test situation so we're actually testing latest changes to the code

@Charles-Gagnon
Copy link
Contributor

Charles-Gagnon commented Nov 7, 2022

Also it would be good to add a .vscode folder with the recommended extensions for these samples (AF and Powershell extensions at least, not sure if there's any more that you've found helpful) as well as other things like launch targets. The goal should be that someone can open VS Code to the samples-powershell folder and directly run the samples from there using F5 and be able to debug their function.

See https://github.com/Azure/azure-functions-sql-extension/pull/458/files#diff-5901d1ff3dfb2f6d0846ab6a4aeab48d916c3a07b3074de578cb760e3be9bb50R3 for an example, you can create a new Powershell function project to get the default stuff from there and then modify it if needed for the samples.

@chlafreniere
Copy link
Contributor

How about including extension recommendations for the PS + Azure functions extensions?

@VasuBhog
Copy link
Contributor Author

VasuBhog commented Nov 7, 2022

  1. Make sure you merge in latest from main to remove the serialization changes
  2. If you haven't been able to verify that it's using the locally built extension DLL during tests (both locally and in the pipeline) then what would probably be best is to remove Powershell from the list of supported languages and then follow up with a PR that actually enables the tests (along with any changes necessary to run using the locally built DLL). This will make that simpler to review and get this in so that others can at least see the various samples and use them while giving you time to figure out the test situation so we're actually testing latest changes to the code

@Charles-Gagnon, after discussion with @lucyzhang929 we found that the local and pipeline are using the latest preview extension bundle.

Here are links to the latest pipeline runs that indicate we are using the correct latest version (4.3.1) of the Preview Extension bundle:
https://mssqltools.visualstudio.com/CrossPlatBuildScripts/_build/results?buildId=178320&view=logs&j=911909e3-f132-51e7-cb10-4a0c4ab1d143&t=c2542b5c-8cb4-5274-ab21-603e725c31e3&l=13289

https://mssqltools.visualstudio.com/CrossPlatBuildScripts/_build/results?buildId=178320&view=logs&j=911909e3-f132-51e7-cb10-4a0c4ab1d143&t=ad6b2e8f-d421-501d-db33-e4ace593a6cb&l=10

Lucy and I discussed steps locally that could be documented further to test locally built DLL:

  1. run dotnet build in root of the repo
  2. Copy the DLL in src/bin/Debug/netstandard2.0 to the extension bundle (samples you want to test - step 3).
  3. Go to the directory of the extension bundle you want to change - for example azure-functions-sql-extension/samples/samples-powershelland usefunc GetExtensionBundlePath` to see where to copy the DLL found in step 2 into.
  4. Copy the DLL from step 2 to the path found in step 3 and run Dotnet test at the root of the repo as it should use the latest builds then.

@Charles-Gagnon
Copy link
Contributor

It's not the latest version of the bundle I'm worried about, it's the version of the extension DLL that is being loaded by the functions during tests. It doesn't help a lot to be using the one from the bundle since the tests should be verify that the code in the repo at the commit the test is running against still works as expected.

  1. Have you verified that Lucy's change to copy the DLL to the bundle in the pipeline also works for the Powershell tests? https://github.com/Azure/azure-functions-sql-extension/blob/main/builds/azure-pipelines/template-steps-build-test.yml#L74 It should, but let's make 100% sure so at least we can rely on the pipeline to be accurate
  2. Documenting manual steps to run the tests locally with the locally built DLL is fine for now but please make an issue for us to follow up with making that done automatically - possibly even embedded as a startup task in the tests themselves.

@VasuBhog
Copy link
Contributor Author

VasuBhog commented Nov 7, 2022

How about including extension recommendations for the PS + Azure functions extensions?

@chlafreniere Pushed latest commit to include recommendations for PS and Az Func.

@lucyzhang929
Copy link
Contributor

Based on the logging from Azure Functions in the pipeline run Vasu linked above, it is loading the extension bundle from the same folder we copy the sql dll into. Although to be 100% sure, maybe we can add some temporary logging and ensure that the logs are being printed in the pipeline.

Automatically copying over the latest dll to our local extension bundle would be great.

Copy link
Contributor

@Charles-Gagnon Charles-Gagnon left a comment

Choose a reason for hiding this comment

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

Changes look good, but you have some test failures now. Please make sure those are addressed before merging in.

@VasuBhog VasuBhog merged commit af99025 into main Nov 8, 2022
@VasuBhog VasuBhog deleted the vabhog/addPSSamplesTests branch November 8, 2022 03:43
Copy link
Contributor

@lucyzhang929 lucyzhang929 left a comment

Choose a reason for hiding this comment

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

Few other samples that have incorrect comments.

# Write to the Azure Functions log stream.
Write-Host "PowerShell function with SQL Output Binding processed a request."

# Update req_query with the body of the request
Copy link
Contributor

Choose a reason for hiding this comment

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

Update comment

# Write to the Azure Functions log stream.
Write-Host "PowerShell function with SQL Output Binding processed a request."

# Update req_query with the body of the request
Copy link
Contributor

Choose a reason for hiding this comment

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

Update comment

# Write to the Azure Functions log stream.
Write-Host "PowerShell function with SQL Output Binding processed a request."

# Update req_body with the body of the request
Copy link
Contributor

Choose a reason for hiding this comment

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

Update comment

# Write to the Azure Functions log stream.
Write-Host "PowerShell function with SQL Output Binding processed a request."

# Update req_body with the body of the request
Copy link
Contributor

Choose a reason for hiding this comment

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

Update comment

# Write to the Azure Functions log stream.
Write-Host "PowerShell function with SQL Output Binding processed a request."

# Update req_body with the body of the request
Copy link
Contributor

Choose a reason for hiding this comment

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

Update comment

# Write to the Azure Functions log stream.
Write-Host "PowerShell function with SQL Output Binding processed a request."

# Update req_body with the body of the request
Copy link
Contributor

Choose a reason for hiding this comment

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

Update comment

# Write to the Azure Functions log stream.
Write-Host "PowerShell function with SQL Output Binding processed a request."

# Update req_body with the body of the request
Copy link
Contributor

Choose a reason for hiding this comment

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

Update comment

PBBlox pushed a commit to PBBlox/azure-functions-sql-extension that referenced this pull request Apr 6, 2025
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.

4 participants