search

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
34.88k stars 9.85k forks source link

Refactor project template tests to make them more consistent and reduce repetition #42188

Open DamianEdwards opened 2 years ago

DamianEdwards commented 2 years ago

From PR comment: https://github.com/dotnet/aspnetcore/pull/42143#issuecomment-1154757891

@HaoK: Not strongly suggesting you do this, but seeing all the instances of SkipOnHelix, and all the logic around computing launch settings. I started wondering if the https tests were split into derived (class i.e. MvcHttpsTemplateTest), it might reduce some of the duplication, as you could have a property controlling whether all the tests were doing Https or not rather than having 2 permutations of every test, and could move the SkipOnHelix to the https class as well. Food for thought anyways

@DamianEdwards: Yeah this is kind of the problem with these tests is they grow so organically over time and it's easiest to just copy/paste/add-dimension, not to mention that often when there's a Helix issue the tests get tactically refactored to allow disabling only the affected tests on Helix. They could certainly be refactored in places but honestly there's a time/effort-element here as well.

@HaoK: Yeah I think maybe we should just have a chore/tracking issue to refactor the tests, definitely shouldn't do it in this PR, but I think these are getting near that unwieldy point where a refactor pass might make sense soon, especially if any more permutations are added

SteveSandersonMS commented 2 years ago

@mkArtakMSFT Do you have enough context on this to consider whether it should be scheduled for .NET 7? Not sure if there's a particular milestone where you'd like to do infrastructure work.

ghost commented 2 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.

ghost commented 1 year 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.

mkArtakMSFT commented 1 year ago

Moving back to the Backlog as the value add from this change is not too high.