[Testing] Remove IgnoreIfPlatforms usage on UITests #22526
Conversation
jsuarezruiz
requested review from
StephaneDelcroix,
tj-devel709,
PureWeen and
jfversluis
and removed request for
StephaneDelcroix and
tj-devel709
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/CarouselViewUITests.LoopNoFreeze.cs
Outdated
Show resolved
Hide resolved
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/CarouselViewUITests.LoopNoFreeze.cs
Outdated
Show resolved
Hide resolved
| @@ -16,10 +16,9 @@ public CarouselViewNoItemTemplate(TestDevice device) | |||
| // Issue12777 (src\ControlGallery\src\Issues.Shared\Issue12777.cs | |||
| [Test] | |||
| [Category(UITestCategories.CarouselView)] | |||
| [FailsOnWindows] | |||
There was a problem hiding this comment.
Should we change these attributes to require a description? Not that this is something that you're doing right now, but imo people should add a description as to why they're adding this.
There was a problem hiding this comment.
I've been thinking about it and if we are going to ignore tests for some reason, it makes sense to add a brief description indicating why.
There was a problem hiding this comment.
Applied changes, the ignore reason is now mandatory.
| @@ -1,4 +1,5 @@ | |||
| using NUnit.Framework; | |||
| #if ANDROID | |||
There was a problem hiding this comment.
For this one (and other #if statements) should we consider not adding this? Ideally tests run for all platforms unless there is something REALLY specific which I don't think should be a lot of tests. What if we just enable them, will they run fine on other platforms as well and suddenly get a lot of extra tests "for free"?
There was a problem hiding this comment.
Maybe if we don't want to fix that right now then let's make sure there is an issue for it to go over these and test that
There was a problem hiding this comment.
Agree, but we need to review every case. I mean, there are many cases that just was validating something in one specific platform and the test only have the reference snapshot for that platform. We just need to generate the rest and validate it.
I can do that in this PR if we want, or create another issue an review it there.
| @@ -46,10 +46,9 @@ public void KeepItemsInView() | |||
| // KeepScrollOffset (src\Compatibility\ControlGallery\src\Issues.Shared\CollectionViewItemsUpdatingScrollMode.cs) | |||
| [Test] | |||
| [Category(UITestCategories.CollectionView)] | |||
| [FailsOnAllPlatforms] | |||
There was a problem hiding this comment.
What is the difference between Ignore that I have seen on other tests and FailsOnAllPlatforms?
There was a problem hiding this comment.
Internally, no differences. However, it can cause confusion so I have made changes to always follow the same pattern using FailsOn with a description.
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue16561.cs
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| #if ANDROID | ||
| public class FailsOnAndroid : IgnoreAttribute |
There was a problem hiding this comment.
Don't we have this in a different place too? Can we move it to somewhere so it's shared maybe?
|
Ignored golden tests, ui tests verifying an snapshot with a reference one, on macOS for now Until implement VerifyScreenshot on Catalyst. |
Description of Change
Remove IgnoreIfPlatforms usage on UITests. The changes should reduce the time needed to pass UI tests.
Issues Fixed
Fixes #22525