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

Load functions trying to find files in snippets test runner #3932

Closed
miqm opened this issue Aug 9, 2021 · 10 comments · Fixed by #3992
Closed

Load functions trying to find files in snippets test runner #3932

miqm opened this issue Aug 9, 2021 · 10 comments · Fixed by #3992
Assignees
Labels
bug Something isn't working

Comments

@miqm
Copy link
Collaborator

miqm commented Aug 9, 2021

@miqm - I thought this is a simple fix, but I cannot get the tests working.

  1. Complains about missing file
  2. main.combined.bicep is always generated incorrectly

I am not sure what I am doing wrong.
image
image
image

Originally posted by @rajyraman in #3919 (comment)

@ghost ghost added the Needs: Triage 🔍 label Aug 9, 2021
@miqm miqm self-assigned this Aug 9, 2021
@miqm miqm added the bug Something isn't working label Aug 9, 2021
@anthony-c-martin
Copy link
Member

anthony-c-martin commented Aug 9, 2021

I think part of the problem is the missing } in placeholder 3. It should be /*${3:'REQUIRED'}*/ rather than /*${3:'REQUIRED'*/.

EDIT: That explains the second error, not the first.

@anthony-c-martin
Copy link
Member

One way you could solve the first problem is by adding a JSON file to the same SnippetTemplates directory and referencing it with:

json(loadTextContent(/*${3:'./res-logic-app-file-workflow.json'}*/'./res-logic-app-file-workflow.json'))

@miqm
Copy link
Collaborator Author

miqm commented Aug 9, 2021

Now that I looked closer is that it should be done in this way. However I didn't test it if it works - I remember that I needed to do some work to get files to be copied to test directory for proper referencing.

@rajyraman can you check if @anthony-c-martin suggestions work?

@anthony-c-martin
Copy link
Member

My suggestion is taking advantage of the fact that .json files get copied to the output directory for the snippet tests. It's definitely a little hacky, and I'm not fully confident it'll work:

<EmbeddedResource Include="./Completions/SnippetTemplates/**/*.json" LogicalName="$([System.String]::new('Completions/SnippetTemplates/%(RecursiveDir)%(Filename)%(Extension)').Replace('\', '/'))" />

The other potential option is to add a similar check to

if (prefix == "module")
{
var paramUri = new Uri("file:///param.bicep");
files.Add(paramUri, "param myParam string = 'test'");
}

e.g. something like:

if (prefix == "res-logic-app-file")
{
     var requiredContents = @"{
     ""definition"": {}
}";
     files.Add(new Uri("file:///REQUIRED"), requiredContents); 
}

@anthony-c-martin
Copy link
Member

@bhsubra do you have any other suggestions?

@miqm
Copy link
Collaborator Author

miqm commented Aug 9, 2021

In https://github.com/Azure/bicep/main/src/Bicep.Core.Samples/Bicep.Core.Samples.csproj#L13 I added that all files in assets subdir are included in output, perhaps something similar we could do for snippet integration tests.

@bhsubra
Copy link
Contributor

bhsubra commented Aug 9, 2021

@bhsubra do you have any other suggestions?

I tried below:

if (prefix == "module") 
{ 
   var paramUri = new Uri("file:///param.bicep"); 
   files.Add(paramUri, "param myParam string = 'test'"); 
} 

This won't work as it hits below code path and tries to load file from local path:

Bicep.Core.dll!Bicep.Core.Semantics.Namespaces.SystemNamespaceSymbol.LoadTextContentTypeBuilder(Bicep.Core.Semantics.IBinder binder, Bicep.Core.FileSystem.IFileResolver fileResolver, Bicep.Core.Diagnostics.IDiagnosticWriter diagnostics, System.Collections.Immutable.ImmutableArray<Bicep.Core.Syntax.FunctionArgumentSyntax> arguments, System.Collections.Immutable.ImmutableArray<Bicep.Core.TypeSystem.TypeSymbol> argumentTypes) Line 466	C#

@miqm, I haven't tried option #1 suggested by @anthony-c-martin . Did that work for you?

@rajyraman
Copy link
Contributor

@anthony-c-martin - I tried your suggestion, but it did not work. Not sure why it is looking in root folder.
image

image

@rajyraman
Copy link
Contributor

@miqm - I am still getting this error even after rebasing. It is complaining about TEST_OUTPUT_DIR folder and not about root folder now.
image

@miqm
Copy link
Collaborator Author

miqm commented Aug 12, 2021

It's because test suite is looking for file called REQUIRED. We had discussion and snippet code should be compilable. You probably need to change the second 'REQUIRED' (the one not in comment) to logicApp.json

Did you push your changes to repo so I could checkout?

@ghost ghost locked as resolved and limited conversation to collaborators May 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants