Disable test recently re-enabled BindingUpdatesFromInteractiveRefresh #28314
Conversation
| @@ -26,7 +26,7 @@ public void BindingUpdatesFromProgrammaticRefresh() | |||
| App.Tap("StopRefreshing"); | |||
| App.WaitForElement("IsNotRefreshing"); | |||
| } | |||
| #if TEST_FAILS_ON_CATALYST //Scroll actions cannot be performed on the macOS test server | |||
| #if TEST_FAILS_ON_CATALYST || TEST_FAILS_ON_WINDOWS //Scroll actions cannot be performed on the macOS test server | |||
There was a problem hiding this comment.
The preprocessor condition now includes TEST_FAILS_ON_WINDOWS, but the inline comment only references macOS. Please update the comment to accurately reflect why the test is also disabled on Windows.
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
| @@ -26,7 +26,7 @@ public void BindingUpdatesFromProgrammaticRefresh() | |||
| App.Tap("StopRefreshing"); | |||
| App.WaitForElement("IsNotRefreshing"); | |||
| } | |||
| #if TEST_FAILS_ON_CATALYST //Scroll actions cannot be performed on the macOS test server | |||
| #if TEST_FAILS_ON_CATALYST || TEST_FAILS_ON_WINDOWS //Scroll actions cannot be performed on the macOS test server | |||
There was a problem hiding this comment.
The message seems not correct. The scroll actions are implement in both, Catalyst and Windows. The test was fixed in Catalyst using the App.DragCoordinates extension method instead App.ScrollDown because sometimes failed (apparently only on CI and requires investigation). On Windows, noticed that failed using the DragCoordinates and applied a small change to fix it here https://github.com/dotnet/maui/pull/28306/files#diff-a4088a095757b5ff1e88b995d447f75adfd3f5afd469cf03ac4bd90126e30f3d
Wouldn't it be better to mark them as FlakyTest and leave a comment with an issue link to re-enable it?
There was a problem hiding this comment.
I'd rather just get rid of the FlakeyTestAttribute
I don't think it's a the safest way to make sure we are addressing tests that start becoming flakey. We don't have any motivation really to just ignore tests in most cases.
In this case, this is a test that was re-added in around 2 weeks ago because it was already flakey
So it's not really a new test
it was already a disabled test and now I'm just re-disabling it, because when it was originally added it was wrong
So this PR is essentially just a revert
If we look at the graph for this test it's been failing since it was originally merged
Description of Change
#28137 re-enabled this test a couple of weeks ago and it's still not 100 percent. This test doesn't represent a change/break in behavior