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

Flaky test: Template_Produces_The_Right_Set_Of_FilesAsync #32406

Closed
JunTaoLuo opened this issue May 4, 2021 · 15 comments · Fixed by #33272 or #34213
Closed

Flaky test: Template_Produces_The_Right_Set_Of_FilesAsync #32406

JunTaoLuo opened this issue May 4, 2021 · 15 comments · Fixed by #33272 or #34213
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Done This issue has been fixed test-failure test-fixed
Milestone

Comments

@JunTaoLuo
Copy link
Contributor

Logs:

Failed Templates.Test.BaselineTest.Template_Produces_The_Right_Set_Of_FilesAsync(arguments: "new blazorserver -au SingleOrg", expectedFiles: ["App.razor", "appsettings.Development.json", "appsettings.json", "Program.cs", "Startup.cs", ...]) [15 m]
  Error Message:
   System.TimeoutException : Process proc dotnet new "web" --debug:disable-sdk-templates --debug:custom-hive "/home/helixbot/work/B9FA0A66/w/BF9D0A69/e/Hives/.templateEngine" timed out after 00:15:00.
  Stack Trace:
     at Templates.Test.Helpers.TemplatePackageInstaller.RunDotNetNew(ITestOutputHelper output, String arguments) in /_/src/ProjectTemplates/Shared/TemplatePackageInstaller.cs:line 85
   at Templates.Test.Helpers.TemplatePackageInstaller.VerifyCannotFindTemplateAsync(ITestOutputHelper output, String templateName) in /_/src/ProjectTemplates/Shared/TemplatePackageInstaller.cs:line 146
   at Templates.Test.Helpers.TemplatePackageInstaller.InstallTemplatePackages(ITestOutputHelper output) in /_/src/ProjectTemplates/Shared/TemplatePackageInstaller.cs:line 110
   at Templates.Test.Helpers.TemplatePackageInstaller.EnsureTemplatingEngineInitializedAsync(ITestOutputHelper output) in /_/src/ProjectTemplates/Shared/TemplatePackageInstaller.cs:line 64
   at Templates.Test.Helpers.ProjectFactoryFixture.GetOrCreateProject(String projectKey, ITestOutputHelper output) in /_/src/ProjectTemplates/Shared/ProjectFactoryFixture.cs:line 28
   at Templates.Test.BaselineTest.Template_Produces_The_Right_Set_Of_FilesAsync(String arguments, String[] expectedFiles) in /_/src/ProjectTemplates/test/BaselineTest.cs:line 86
--- End of stack trace from previous location ---

Build:
https://dev.azure.com/dnceng/public/_build/results?buildId=1119946&view=results

@JunTaoLuo JunTaoLuo added test-failure area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels May 4, 2021
@mkArtakMSFT mkArtakMSFT added this to the Next sprint planning milestone May 6, 2021
@ghost
Copy link

ghost commented May 6, 2021

Thanks for contacting us.

We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@HaoK HaoK self-assigned this Jun 4, 2021
@HaoK
Copy link
Member

HaoK commented Jun 4, 2021

This should be an easy fix as looks like I just need to update the baselines to remove the react-redux that's been deleted

@ghost ghost added the Working label Jun 4, 2021
@ghost ghost added Done This issue has been fixed and removed Working labels Jun 4, 2021
@HaoK HaoK reopened this Jun 4, 2021
@HaoK HaoK added the test-fixed label Jun 4, 2021
@HaoK
Copy link
Member

HaoK commented Jun 7, 2021

@dotnet/aspnet-build can I preemptively reenable this test since its job is to verify that the expected files are on disk, its quite easy to break this whenever template updates are made, so its better for this to be enabled now that its fixed as opposed to waiting 30 days, I almost forgot about updating this as part of #33353

Its not really doing anything that is likely to be flaky

@dougbu
Copy link
Member

dougbu commented Jun 7, 2021

I understand the original problem has been fixed. However I'm hesitant to reenable anything in the templates suite given flakiness in dotnet new, dotnet restore, and so on. My preference is also to stick with the agreed-upon process and avoid taking things on a case-by-case basis.

@HaoK
Copy link
Member

HaoK commented Jun 8, 2021

I'm not sure how that changes anything, after 30 days, even if this test were to fail due to random dotnet new/restore issues that are completely unrelated to this test, would we just keep it in quarantine forever?

@dougbu
Copy link
Member

dougbu commented Jun 8, 2021

would we just keep it in quarantine forever?

If we have tests that fail more than twice a year, they aren't providing value.

@HaoK
Copy link
Member

HaoK commented Jun 8, 2021

If we have tests that fail more than twice a year, they aren't providing value.

I'm not sure what you are really arguing here, taken to the extreme we can just have no tests and no coverage, end to end tests are basically never going to have that kind of pass rate, so are you suggesting we just give up and delete the template tests since dotnet new and dotnet restore will never provide value by that metric?

@dougbu
Copy link
Member

dougbu commented Jun 8, 2021

I'm not sure what you are really arguing here

I'm arguing the template tests aren't providing much value at the moment and we shouldn't make the problem worse by speeding the un-quarantining process.

I don't have a specific recommendation to improve what's going on w/ the template tests. #33230 for example won't help here unless this is specifically a Blazor test (side note: I'm more worried retries will hide issues than solve them).

@HaoK
Copy link
Member

HaoK commented Jun 8, 2021

Well longer term we can just set a strategy in the new helix retry stuff to say something like 70% passrate for template tests is expected, its not great, but its better than just ignoring real regression/failures when the tests go to 0% consistently like we've been doing

@TanayParikh
Copy link
Contributor

image

97% pass rate over past month, not eligible for un-quarantining unfortunately.

@HaoK
Copy link
Member

HaoK commented Jul 7, 2021

This is exactly why I said this test should have been unquarantined when the fix when in, the full red is due to the templates changes, the other failures on 6/25 6/24 look to be general infrastructure failures: that would have caused all project template tests to fail,

'System.InvalidOperationException : Failed to uninstall previous templates. The template 'web' could still be found.'
System.TimeoutException : Process proc dotnet new "web" --debug:disable-sdk-templates --debug:custom-hive "/private/tmp/helix/working/ABC3094A/w/9B150853/e/Hives/.templateEngine" timed out after 00:15:00.

There's nothing to do in terms of flaky fixes for this test specifically as this test is not flaky, its the project templates infrastructure that is flaky

But for now, assigning this to @DamianEdwards to update https://github.com/dotnet/aspnetcore/blob/main/src/ProjectTemplates/test/template-baselines.json to react to the template changes due to the expected file set being different now

@DamianEdwards
Copy link
Member

@HaoK is there a tool for generating that file or is hand-authored?

@HaoK
Copy link
Member

HaoK commented Jul 8, 2021

Thanks @DamianEdwards reopening since we keep these around to track quarantined tests, marking this as fixed again

@HaoK HaoK reopened this Jul 8, 2021
@HaoK HaoK added the test-fixed label Jul 8, 2021
@HaoK
Copy link
Member

HaoK commented Jul 8, 2021

@dotnet/aspnet-build I think we should preemptively unquarantine this test as its special and is verifying what files we lay down on disk, this test basically should never be quarantined

@wtgodbe
Copy link
Member

wtgodbe commented Jul 8, 2021

I buy that argument, feel free to open a PR

@ghost ghost locked as resolved and limited conversation to collaborators Aug 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Done This issue has been fixed test-failure test-fixed
Projects
None yet
7 participants