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

[Microsoft.Build] Investigate non-deterministic test behavior #6781

Closed
YuliiaKovalova opened this issue Jan 13, 2023 · 10 comments
Closed

[Microsoft.Build] Investigate non-deterministic test behavior #6781

YuliiaKovalova opened this issue Jan 13, 2023 · 10 comments
Assignees
Labels
Iteration:2023June Priority:1 Work that is critical for the release, but we could probably ship without

Comments

@YuliiaKovalova
Copy link
Member

YuliiaKovalova commented Jan 13, 2023

Product

dotnet CLI (dotnet new)

Describe The Bug

When a new item template is instantiated, the warning "Failed to evaluate bind symbol 'langVersion', it will be skipped." is generated sometimes.

Identify what triggers this scenario.
Can be reproduced in the src/Tests/dotnet-new.Tests/DotnetClassTemplateTests.cs tests

To Reproduce

Steps:

  1. Go to src/Tests/dotnet-new.Tests/DotnetClassTemplateTests.cs
  2. Comment custom scrubber that removes the warning message
  3. Run tests a couple of times.

dotnet Info

release/7.2.0xx

@YuliiaKovalova
Copy link
Member Author

Tips for investigation:
Enable debugging logging on the pipeline

@YuliiaKovalova YuliiaKovalova added the triaged The issue was evaluated by the triage team, placed on correct area, next action defined. label Jan 25, 2023
@GangWang01
Copy link
Member

GangWang01 commented Feb 27, 2023

It was not able to reproduce with release/7.0.2xx after running tests many times locally or in the pipeline.
When we decided to remove scrubbing the warning for further observation in main branch, it is finally reproduced. And I found the warning happened to the following cases of DotnetCSharpClassTemplatesTest which only have shortname. Need to investigate further.

  • [InlineData("class")]
  • [InlineData("interface")]
  • [InlineData("record")]
  • [InlineData("struct")]
  • [InlineData("enum")]

@YuliiaKovalova YuliiaKovalova added Priority:3 Work that is nice to have and removed Iteration:2023February labels May 18, 2023
@GangWang01
Copy link
Member

GangWang01 commented Jun 19, 2023

In the item templates mentioned above, the bind symbol langVersion binding to msbuild:LangVersion has different values from the context evaluation of the project without property LangVersion that is done by different versions of the package Microsoft.Build.
For the created library project without language version in the tests, the evaluation result for the bind symbol langVersion in different branches with different versions of the package Microsoft.Build is as below.

Branch Package Version Evaluation Result
release/7.0.2xx Microsoft.Build 17.5.2 11.0
main Microsoft.Build 17.6.0-preview-23122-03 or above null

Need to confirm the behaviors of Microsoft.Build are right or wrong.

Here is the direct call from Microsoft.Build https://github.com/dotnet/sdk/blob/6c1b1f5f9e0035df7706467c63b99d264578fab0/src/Cli/dotnet/commands/dotnet-new/MSBuildEvaluation/MSBuildEvaluator.cs#L262 .

@JanKrivanek
Copy link
Member

Binlog might be very helpful here to aid investigation.
I'd suggest running the repro with MSBUILDDEBUGENGINE envvar set to 1 (more details: https://github.com/dotnet/msbuild/blob/main/documentation/wiki/Providing-Binary-Logs.md)

@YuliiaKovalova YuliiaKovalova transferred this issue from dotnet/templating Jun 19, 2023
@YuliiaKovalova YuliiaKovalova changed the title Investigate non-deterministic test behavior (Caused by MSBuild.Build changes) Investigate non-deterministic test behavior Jun 19, 2023
@YuliiaKovalova YuliiaKovalova changed the title (Caused by MSBuild.Build changes) Investigate non-deterministic test behavior [Microsoft.Build] Investigate non-deterministic test behavior Jun 19, 2023
@JanKrivanek JanKrivanek removed the triaged The issue was evaluated by the triage team, placed on correct area, next action defined. label Jun 19, 2023
@GangWang01
Copy link
Member

Attached the binlog binlogs.zip.
Running test [InlineData("class")], latest_7.0.2xx.binlog is from release/7.0.2xx and latest_main.binlog is from main.

@JanKrivanek
Copy link
Member

Seems langVersion property is not populated on latest main:

image

This seems to be a culprit: https://github.com/dotnet/roslyn/blob/b059755627eb8ee45aee083c967bada95583bbfc/src/Compilers/Core/MSBuildTask/Microsoft.CSharp.Core.targets#L6-L30

LangVersion is not set for net8 targetting.
I'll try to see if this is expected

@JanKrivanek
Copy link
Member

@GangWang01 this was confirmed as expected behavior by Roslyn team - the in-development TFM doesn't have langVarsion populated. The built-in templates should already properly handle the absence of the value as the newest (e.g. https://github.com/dotnet/sdk/blob/main/template_feed/Microsoft.DotNet.Common.ProjectTemplates.8.0/content/ConsoleApplication-CSharp/.template.config/template.json), so probably any other code that might break with missing value should handle it gracefuly

@YuliiaKovalova YuliiaKovalova transferred this issue from dotnet/msbuild Jun 20, 2023
@YuliiaKovalova
Copy link
Member Author

@GangWang01 , if you have some time, could you check the output for project templates having the same parameters as in tests?

@GangWang01
Copy link
Member

@GangWang01 , if you have some time, could you check the output for project templates having the same parameters as in tests?

Attached the generated project and debug log by running local custom tests that instantiate Console Application CSharp and Class Library CSharp output.zip without language version specified.

@YuliiaKovalova YuliiaKovalova added Priority:1 Work that is critical for the release, but we could probably ship without Iteration:2023June and removed Priority:3 Work that is nice to have labels Jun 26, 2023
@YuliiaKovalova
Copy link
Member Author

In order to handle this restriction we recommend introducing a generated symbol for langVersion:

    // symbol with langVersion extracted from MSBuild
    "evaluatedLangVersion": {
      "type": "bind",
      "binding": "msbuild:LangVersion",
      "dataType": "string"
    },

    // default langVersion value
    "latestLangVersion": {
      "type": "generated",
      "generator": "constant",
      "parameters": {
        "value": "latest"
      }
    },

    // if "evaluatedLangVersion" is empty, the value will be taken from "latestLangVersion" symbol
    "langVersion": {
      "type": "generated",
      "generator": "coalesce",
      "parameters": {
        "sourceVariableName": "evaluatedLangVersion",
        "fallbackVariableName": "latestLangVersion"
      }
    }, 

For getting more info on generated symbols check out the wiki page: https://github.com/dotnet/templating/wiki/Available-Symbols-Generators .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Iteration:2023June Priority:1 Work that is critical for the release, but we could probably ship without
Projects
None yet
Development

No branches or pull requests

4 participants