dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.44k stars 10.02k forks source link

Reenable taghelpers glob tests #29597

Open HaoK opened 3 years ago

HaoK commented 3 years ago

There are two tests skipped on helix due to the move to running from the publish directory from https://github.com/dotnet/aspnetcore/pull/29449

dougbu commented 3 years ago

Made the description a checklist in case we handle some of these individually. Also added IdentityUI_ScriptTags_FallbackSourceContent_Matches_CDNContent to the list.

Good news is [SkipOnHelix(...)] continues to be used mostly with a list of queues to avoid. Bad news is the tests in this issue are not run at all because $(SKipHelixReadyTest) operates at the project level, skipping all tests that would normally build a Helix payload.

dougbu commented 3 years ago

From #30703 discussion:

One solution would be to add a new property that allows us to run only [SkipOnHelix(...)] tests on the build agents.

Another (ugly) option would be to continue not running those tests at all. Might as well remove them or treat them as manual-only tests☹️

My first suggestion isn't complete because we still want to run tests in projects that do not build Helix payloads. This issue is about finding a way to run the (fortunately small) subset of tests from projects that do build Helix payloads but the particular test won't execute in any Helix queue. That's in addition to running the tests we currently run on the build agents.

HaoK commented 3 years ago

Well this issue is specifically only about the tests I broke as part of helixification work in the unification of publish/functional tests, IdentityUIScriptsTest.IdentityUI_ScriptTags_FallbackSourceContent_Matches_CDNContent is unlikely to be related to that so it probably won't be resolved by this bug.

HaoK commented 3 years ago

I think what you want is another meta issue that is about what to do about specific tests that are skipped on helix (for all queues), I would assume each of those needs to be tracked by its own issue if its something that needs to be followed up on, there's not going to be a general catch all soln for all the different tests that don't work on helix in some way

dougbu commented 3 years ago

there's not going to be a general catch all soln for all the different tests that don't work on helix in some way

Why not❔ My solution would work though it's likely pretty inefficient.

Another catch-all option would be to split the individual tests into separate non-Helix-ready projects. Best case would be a single project, albeit w/ lots of dependencies.

HaoK commented 3 years ago

I mean sure I guess that's possible, but it seems more worthwhile to just fix the tests that we really care about, and the rest just fall into the same bucket as test failures/quarantined tests (debt). There really shouldn't be many tests in this bucket anyways, the plan generally for these to be skipped and have issues filed for them, as its basically follow up work from the helixifaction.

ghost commented 3 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. 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.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

dougbu commented 2 years ago

Note the MVC functionals in the Description remain skipped on Helix and probably shouldn't be checked off. They also fail locally when run from the publish/ folder e.g. dotnet vstest Microsoft.AspNetCore.Mvc.FunctionalTests.dll --TestCaseFilter:"Quarantined!=true|Quarantined=false". (Same command is fine in bin/ folder.)

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 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.

jjonescz commented 1 year ago

FYI, the ViewComponentTagHelpers test needs to be updated as the syntax <vc:generic items-foo="items"></vc:generic> is not supported by the new compiler. See https://github.com/dotnet/razor/issues/8521.

https://github.com/dotnet/aspnetcore/blob/d4be4957c24c7c8b745ade4cbaf290ad9cad1ad2/src/Mvc/test/WebSites/TagHelpersWebSite/Views/Home/ViewComponentTagHelpers.cshtml#L10

https://github.com/dotnet/aspnetcore/blob/d4be4957c24c7c8b745ade4cbaf290ad9cad1ad2/src/Mvc/test/Mvc.FunctionalTests/TagHelpersTest.cs#L61