Skip to content

Fix folder used for Integration tests #386

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.

Sign up for GitHub

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 2 commits into from
Oct 11, 2022
Merged

Fix folder used for Integration tests #386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ea80f6
fix working folder for integration tests
lucyzhang929 Oct 6, 2022
fb3a9e0
fix datetime format
lucyzhang929 Oct 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/Integration/IntegrationTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ protected void StartAzurite()
/// </remarks>
protected void StartFunctionHost(string functionName, SupportedLanguages language, bool useTestFolder = false, DataReceivedEventHandler customOutputHandler = null)
{
string workingDirectory = useTestFolder ? GetPathToBin() : Path.Combine(GetPathToBin(), "SqlExtensionSamples", Enum.GetName(typeof(SupportedLanguages), language));
string workingDirectory = language == SupportedLanguages.CSharp && useTestFolder ? GetPathToBin() : Path.Combine(GetPathToBin(), "SqlExtensionSamples", Enum.GetName(typeof(SupportedLanguages), language));
Copy link
Contributor

@Charles-Gagnon Charles-Gagnon Oct 6, 2022

Choose a reason for hiding this comment

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

Actually - I don't even think we need this anymore if you just update https://github.com/Azure/azure-functions-sql-extension/blob/a20541cfedb4a774395a364454f419f546b5e470/test/Microsoft.Azure.WebJobs.Extensions.Sql.Tests.csproj to also copy the CS bindings from Integration\test-cs into the SqlExtensionSamples folder. There's a bit of extra duplication there with having multiple copies of the functions, but it makes the test code simpler and running the functions from the same place will make it easier to diagnose issues.

I would also suggest when you do that updating the folder to just be generically named TestFunctions

Copy link
Contributor Author

@lucyzhang929 lucyzhang929 Oct 6, 2022

Choose a reason for hiding this comment

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

Yes that makes sense - I'll update that. Thanks Charles!

Copy link
Contributor Author

@lucyzhang929 lucyzhang929 Oct 7, 2022

Choose a reason for hiding this comment

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

One issue I’m running into is that we need to copy the built function files (ex. AddProductsColumnTypes folder containing the function.json file) and not the .cs files. The built function folders are in bin/Debug/netcoreapp3.1/ but I’m not sure if there is a way to only copy those folders into the TestFunctions folder. 


Screen Shot 2022-10-06 at 6 30 29 PM

Another option is to move the functions in the test folder into a separate folder in samples ex. Samples/samples-csharp/InvalidSamples. Then all functions can be built in the same project. This will also make it easier for Java so that we don't have to build two java projects: samples/samples-java and test/Integration/test-java. @Charles-Gagnon @MaddyDev thoughts?

Copy link
Contributor

@Charles-Gagnon Charles-Gagnon Oct 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I'd rather avoid having the invalid (test) functions in the samples - that's just confusing for the users and could lead to someone not paying attention and trying to do something that isn't going to work.

You should be able to just use the same logic we have for adding the items from the sample folder - we just need to special case to not copy over the target folder.

      <_CSharpCopyItems Include="..\samples\samples-csharp\bin\$(Configuration)\$(TargetFramework)\**\*.*" />
      <_CSharpCopyItems Include="$(OutDir)\**\function.json" />
      <_CSharpCopyItems Remove="$(OutDir)\SqlExtensionSamples\**\*.*" />

It's not the prettiest but it'll work and I can't think of a simpler way to do it right now.

Copy link
Contributor Author

@lucyzhang929 lucyzhang929 Oct 11, 2022

Choose a reason for hiding this comment

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

I agree, having the test functions in the samples can be confusing.

I tried copying over the function.json files but the issue is that the function.json files for CSharp refers to the dll needed which would be different for the Samples vs Test functions:
"scriptFile": "../bin/Microsoft.Azure.WebJobs.Extensions.Sql.Tests.dll",.
I also tried copying over the Test.dll but the functions still don't get loaded correctly. I believe it is because of the functions.deps.json file which references the target project and would be different for Samples vs Tests.

For now we will have to stick to two separate folders for CSharp tests (I think Java will also need to separate folders).

Copy link
Contributor

@Charles-Gagnon Charles-Gagnon Oct 11, 2022

Choose a reason for hiding this comment

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

Ah dang, yeah ok let's just leave it like it is then with two completely separate folders. Thanks for looking into that.

if (!Directory.Exists(workingDirectory))
{
throw new FileNotFoundException("Working directory not found at " + workingDirectory);
Expand Down
4 changes: 2 additions & 2 deletions test/Integration/test-js/AddProductColumnTypes/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
module.exports = async function (context, req) {
const product = {
"productId": req.query.productId,
"datetime": Date.now(),
"datetime2": Date.now()
"datetime": new Date().toISOString(),
"datetime2": new Date().toISOString()
};

context.bindings.product = JSON.stringify(product);
Expand Down