From 75b7470d66d9c37dc8b0ec4c95e22e3a6fd489cf Mon Sep 17 00:00:00 2001 From: amrali-eg <32075105+amrali-eg@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:57:48 +0300 Subject: [PATCH 1/6] Retry a refused Cancel, and stop counting every file to trigger it Two defects in the cancellation path, both found by review of the last change. RequestCancel gave up on the first ElementNotEnabledException and then waited for the run to report itself finished - which it could not do for another ten seconds, because the run was still going. The click is now retried, each attempt reacquiring the button, and the wait ends on a click that was accepted, a status saying the run finished on its own, or the timeout. A control that refuses the first five clicks now passes and still interrupts the run; against the previous shape a single refusal failed the phase twice out of two - and failed it with the wrong explanation, reporting that the conversion had finished when it was still writing. The trigger asked RewrittenCount(...) >= 1, which opened all thousand files on every 50 ms probe while EC was rewriting those same files. It now asks AnyRewritten, which stops at the first one. The count is still taken once, after the run has stopped, where it is what the assertions compare. The margin this bought is visible in the phase's own line: cancellation used to land after 38-93 of the thousand files and now lands after 16-60. EC is unchanged. Co-Authored-By: Claude Opus 5 --- .../EncodingChecker.GuiSmoke/EcGuiDriver.cs | 51 +++++++++++-------- .../EncodingChecker.GuiSmoke/SmokeSuite.cs | 14 ++++- 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs b/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs index b0336fb..34b9dbb 100644 --- a/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs +++ b/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs @@ -263,41 +263,48 @@ internal void ProceedThenCancel(AutomationElement review, Func writingHasB /// The button and final status are raced so a fast completion produces a clear failure /// instead of a misleading timeout. If neither appears, the wait retains the last /// automation error. + /// + /// A refused click is retried rather than believed. A control reporting itself + /// not-enabled is the same flag this driver stopped trusting for readiness, and one + /// refusal says nothing about whether the run is over; only the window's own final + /// status does. So each attempt reacquires the button, and the wait ends on a click + /// that was accepted, a run that reported itself finished, or the timeout. /// private void RequestCancel() { - AutomationElement? cancel = null; + bool cancelled = false; WaitUntil( () => { - cancel = FindById(MainWindow, "btnCancel"); - return cancel is not null || ConversionHasFinished(); + AutomationElement? cancel = FindById(MainWindow, "btnCancel"); + + // Gone means the window hid it, which it does only when a run ends - but + // that has to come from the status, not from the button's absence. + if (cancel is null) + return ConversionHasFinished(); + + try + { + Invoke(cancel); + cancelled = true; + return true; + } + catch (Exception ex) when ( + ex is ElementNotEnabledException or ElementNotAvailableException) + { + // Refused this time. The next attempt looks the button up again; if + // the run really has ended, the branch above sees the status say so. + return false; + } }, - "The run offered neither a Cancel button nor a final status."); + "Cancel was never accepted, and the run never reported that it had stopped."); - if (cancel is null) + if (!cancelled) { throw new GuiDriverException( "The conversion finished before cancellation could be exercised."); } - - try - { - Invoke(cancel); - } - catch (Exception ex) when ( - ex is ElementNotEnabledException or ElementNotAvailableException) - { - // Confirm why the button vanished, but do not count ordinary completion as a - // cancellation test. - WaitForOperationOutcome( - () => ConversionHasFinished(), - "Cancel became unavailable without the run reporting that it had stopped."); - - throw new GuiDriverException( - "The conversion finished before cancellation could be exercised."); - } } /// Requires the cancellation request to produce an interrupted run. diff --git a/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs b/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs index 9c52a50..4e143ec 100644 --- a/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs +++ b/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs @@ -422,7 +422,7 @@ private void PhaseI(PhaseContext phase) using var gui = new EcGuiDriver(_app); System.Windows.Automation.AutomationElement review = gui.OpenReview(directory, count); - gui.ProceedThenCancel(review, () => RewrittenCount(directory) >= 1); + gui.ProceedThenCancel(review, () => AnyRewritten(directory)); int rewritten = RewrittenCount(directory); int untouched = count - rewritten; @@ -512,6 +512,18 @@ private void PhaseJ(PhaseContext phase) } /// Files whose byte-order mark has been stripped, so they were written. + /// Whether EC has rewritten anything yet. + /// + /// Asked every 50 ms while the conversion is running, so it stops at the first + /// rewritten file rather than counting them all. Counting opens every file in the + /// directory on each probe, which delays the cancellation this triggers and eats the + /// margin phase I depends on - the count is wanted once, after the run has stopped. + /// + private static bool AnyRewritten(string directory) => + Directory.EnumerateFiles(directory, "file-*.txt") + .Any(path => !StartsWithUtf8Bom(path)); + + /// How many files EC rewrote. Taken after the run, never during it. private static int RewrittenCount(string directory) => Directory.EnumerateFiles(directory, "file-*.txt") .Count(path => !StartsWithUtf8Bom(path)); From 2321b4da2b6a79b44b6cdf47e8211204f44dd178 Mon Sep 17 00:00:00 2001 From: amrali-eg <32075105+amrali-eg@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:17:10 +0300 Subject: [PATCH 2/6] Let a refused Cancel reach the retry loop that keeps the cause The refusal was caught inside the predicate and turned into "not yet". That retried just as often, but the shared loop clears its retained error whenever a probe answers cleanly, so every refusal erased the very thing a timeout would have reported - while the remarks claimed the wait retained it. ElementNotEnabledException derives from InvalidOperationException, which the loop already catches, so the refusal now simply propagates: it is retried and kept. Confirmed by compilation rather than assumed - catching it after InvalidOperationException is CS0160, "a previous catch clause already catches all exceptions of this or of a super type". The behaviour is unchanged where it was already right: a control refusing the first five clicks still passes and still interrupts the run. No control here shows the improved message, because staging one needs a conversion outliving the thirty-second wait, and this workload ends in about ten seconds. The first attempt at such a control was invalid - blinding the completion check made the predicate answer cleanly for the rest of the wait, which cleared the error in both shapes and would have shown no difference for the wrong reason. Also drops a stranded summary left above AnyRewritten by the previous commit. EC is unchanged. Co-Authored-By: Claude Opus 5 --- .../EncodingChecker.GuiSmoke/EcGuiDriver.cs | 24 +++++++++---------- .../EncodingChecker.GuiSmoke/SmokeSuite.cs | 1 - 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs b/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs index 34b9dbb..f9e1173 100644 --- a/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs +++ b/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs @@ -269,6 +269,11 @@ internal void ProceedThenCancel(AutomationElement review, Func writingHasB /// refusal says nothing about whether the run is over; only the window's own final /// status does. So each attempt reacquires the button, and the wait ends on a click /// that was accepted, a run that reported itself finished, or the timeout. + /// + /// The refusal is left to that shared loop rather than caught here. It already + /// retries these errors and keeps the last one, so a wait that does expire can name + /// the refusal it kept hitting; catching it here would retry just as often and + /// report nothing. /// private void RequestCancel() { @@ -284,19 +289,12 @@ private void RequestCancel() if (cancel is null) return ConversionHasFinished(); - try - { - Invoke(cancel); - cancelled = true; - return true; - } - catch (Exception ex) when ( - ex is ElementNotEnabledException or ElementNotAvailableException) - { - // Refused this time. The next attempt looks the button up again; if - // the run really has ended, the branch above sees the status say so. - return false; - } + // A refusal throws out of here into the retry loop, which reacquires the + // button on the next attempt. If the run really has ended by then, the + // branch above sees the status say so. + Invoke(cancel); + cancelled = true; + return true; }, "Cancel was never accepted, and the run never reported that it had stopped."); diff --git a/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs b/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs index 4e143ec..2470f69 100644 --- a/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs +++ b/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs @@ -511,7 +511,6 @@ private void PhaseJ(PhaseContext phase) AssertNoArtifacts(phase.Directory); } - /// Files whose byte-order mark has been stripped, so they were written. /// Whether EC has rewritten anything yet. /// /// Asked every 50 ms while the conversion is running, so it stops at the first From f913700f4b2621651bdacabae1fc0983dfc934c6 Mon Sep 17 00:00:00 2001 From: amrali-eg <32075105+amrali-eg@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:25:16 +0300 Subject: [PATCH 3/6] Stop claiming the retry loop always keeps the cause It keeps the last error only while the probe is still failing: a poll that answers cleanly clears it. Three comments said or implied otherwise, including the parameter's own documentation, which is where the rule belongs. RequestCancel now says the loop can report a refusal if refusals continue to the timeout, and that catching it here would turn every refusal into a clean "not ready" answer - which is precisely what makes the loop drop the cause. The duplicate claim higher in the same remarks is gone. Comments only; no behaviour changes. Co-Authored-By: Claude Opus 5 --- sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs b/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs index f9e1173..1f1f157 100644 --- a/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs +++ b/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs @@ -261,8 +261,7 @@ internal void ProceedThenCancel(AutomationElement review, Func writingHasB /// proving cancellation, so neither is accepted as a successful test. /// /// The button and final status are raced so a fast completion produces a clear failure - /// instead of a misleading timeout. If neither appears, the wait retains the last - /// automation error. + /// instead of a misleading timeout. /// /// A refused click is retried rather than believed. A control reporting itself /// not-enabled is the same flag this driver stopped trusting for readiness, and one @@ -270,10 +269,10 @@ internal void ProceedThenCancel(AutomationElement review, Func writingHasB /// status does. So each attempt reacquires the button, and the wait ends on a click /// that was accepted, a run that reported itself finished, or the timeout. /// - /// The refusal is left to that shared loop rather than caught here. It already - /// retries these errors and keeps the last one, so a wait that does expire can name - /// the refusal it kept hitting; catching it here would retry just as often and - /// report nothing. + /// The refusal is left to that shared loop rather than caught here. The loop retries + /// it and can report it if refusals continue until the timeout. Catching it here + /// would turn every refusal into a clean "not ready" answer, which is what makes the + /// loop drop the cause. /// private void RequestCancel() { @@ -847,8 +846,10 @@ private static AutomationElement WaitForElement( ?? throw Expired(timeoutMessage, lastError); /// - /// The last error retried before giving up. A probe that threw every time is the - /// likeliest reason a wait expired, and it is what a bare timeout cannot report. + /// The last error retried, when the probe was still failing at the end. A poll that + /// answers cleanly clears it, so this reports the cause of a wait that kept throwing + /// rather than one that simply never became true - which is the case a bare timeout + /// cannot explain by itself. /// There is deliberately no overload without it, so a wait that reports a timeout /// cannot leave the cause out by accident. Only a caller with nothing to report /// discards it, and no wait in this driver currently does. From 1534d6c3142054509cb929daa0781f43b04ecb1b Mon Sep 17 00:00:00 2001 From: amrali-eg <32075105+amrali-eg@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:29:41 +0300 Subject: [PATCH 4/6] Read the target encoding once, and compare it the way the driver does Check builds its message whether or not the assertion fails, so each of these asked the window for the target twice on every run: once to test it, once to describe it. That drove the control on passing runs, and let a transient automation error inside a message fail a check whose condition had held - the defect the driver added Safely() to avoid. It could also report a value the assertion never tested, since the second read is a separate observation. The comparison was ordinal while SelectCombo and RequireDefaultTarget both use OrdinalIgnoreCase, so a control reporting "UTF-8" would have satisfied the driver and failed the phase. Both now read once into a local and compare case-insensitively, and the message reports that same reading. The assertions still catch what they exist for: with SetTargetEncoding reduced to a no-op, phase A fails twice out of two with "The target encoding did not change: utf-8". EC is unchanged. Co-Authored-By: Claude Opus 5 --- sources/EncodingChecker.GuiSmoke/SmokeSuite.cs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs b/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs index 2470f69..3130bd1 100644 --- a/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs +++ b/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs @@ -177,13 +177,20 @@ private void PhaseA(PhaseContext phase) // otherwise drives that control. Exercise it here, where the run is over and this // phase is about to prove no bytes moved: a target that could not be changed would // pass every other phase unnoticed. + // Read once and report that same reading: Check takes a message that is built + // whether or not it fails, so asking the window again inside it would drive the + // control on every passing run and could describe a value the assertion never + // tested. Compared the way the driver compares it, so the two cannot disagree + // over a control label's casing. gui.SetTargetEncoding("us-ascii"); - Check(gui.TargetEncoding() == "us-ascii", - $"The target encoding did not change: {gui.TargetEncoding()}"); + string changed = gui.TargetEncoding(); + Check(changed.Equals("us-ascii", StringComparison.OrdinalIgnoreCase), + $"The target encoding did not change: {changed}"); gui.SetTargetEncoding("utf-8"); - Check(gui.TargetEncoding() == "utf-8", - $"The target encoding did not change back: {gui.TargetEncoding()}"); + string restored = gui.TargetEncoding(); + Check(restored.Equals("utf-8", StringComparison.OrdinalIgnoreCase), + $"The target encoding did not change back: {restored}"); AssertSameFiles(before, Snapshot(directory)); AssertNoArtifacts(directory); From 87a3699846803fb1f11f9ba7a272aef3f43139db Mon Sep 17 00:00:00 2001 From: amrali-eg <32075105+amrali-eg@users.noreply.github.com> Date: Thu, 10 Sep 2026 01:01:09 +0300 Subject: [PATCH 5/6] Let the final status decide whether cancellation happened Delivering a click is not proof it took effect, and failing to deliver one is not proof it did not. RequestCancel tracked a cancelled flag and threw when it was false, so a click that landed and then failed - because the window hid the button in response, which is exactly what a successful cancel looks like - reported "The conversion finished before cancellation could be exercised" while the status bar read "Conversion stopped". EC writes that headline only for a run that was stopped, so the failure contradicted its own evidence. The flag is gone. RequestCancel asks and then waits for the run to report something; WaitForStoppedConversion reads what it reported. Conversion stopped means cancellation worked, Conversion complete means it was not exercised, and no final status at all is the timeout. The two ways a click can fail are no longer treated alike. A refusal happens before anything is delivered, so it is retried. An element that vanishes mid-call, or a COM failure, may follow a click that did land, so the attempt stops rather than pressing the button a second time. SelectedName absorbs a provider's null instead of handing it out, and every comparison of its result uses the static string.Equals overload. The instance call introduced a NullReferenceException where the previous == could not throw. Controls: a click that lands and then throws now passes twice out of two and still interrupts the run, where the previous shape failed twice with the false message. Five refusals followed by acceptance still cancel. A no-op target setter still fails phase A with "The target encoding did not change: utf-8". EC is unchanged. Co-Authored-By: Claude Opus 5 --- .../EncodingChecker.GuiSmoke/EcGuiDriver.cs | 86 +++++++++++-------- .../EncodingChecker.GuiSmoke/SmokeSuite.cs | 4 +- 2 files changed, 53 insertions(+), 37 deletions(-) diff --git a/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs b/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs index 1f1f157..b5d98de 100644 --- a/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs +++ b/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs @@ -76,7 +76,7 @@ private void RequireDefaultTarget() { string target = TargetEncoding(); - if (!target.Equals("utf-8", StringComparison.OrdinalIgnoreCase)) + if (!string.Equals(target, "utf-8", StringComparison.OrdinalIgnoreCase)) { throw new GuiDriverException( $"The target encoding opens on '{target}', not 'utf-8'. Every phase " @@ -263,45 +263,58 @@ internal void ProceedThenCancel(AutomationElement review, Func writingHasB /// The button and final status are raced so a fast completion produces a clear failure /// instead of a misleading timeout. /// - /// A refused click is retried rather than believed. A control reporting itself - /// not-enabled is the same flag this driver stopped trusting for readiness, and one - /// refusal says nothing about whether the run is over; only the window's own final - /// status does. So each attempt reacquires the button, and the wait ends on a click - /// that was accepted, a run that reported itself finished, or the timeout. + /// Whether cancellation happened is not decided here. Delivering a click is not + /// proof it took effect, and failing to deliver one is not proof it did not: the + /// window's own final status is the only thing that separates a run that was stopped + /// from one that finished on its own, and + /// reads it. This method's job is to ask, and to wait until the run has reported + /// something. /// - /// The refusal is left to that shared loop rather than caught here. The loop retries - /// it and can report it if refusals continue until the timeout. Catching it here - /// would turn every refusal into a clean "not ready" answer, which is what makes the - /// loop drop the cause. + /// The two ways a click can fail mean different things. A refusal - the control + /// reporting itself not-enabled - happens before anything is delivered, so trying + /// again is safe and right. An element that disappears mid-call, or a COM failure, + /// is what a click that *did* land looks like when the window hides the button in + /// response; it may equally have failed before delivering. Clicking again there + /// would be a fresh action rather than a retry, so the attempt stops and the status + /// is left to say what happened. /// private void RequestCancel() { - bool cancelled = false; + bool clickMayHaveLanded = false; WaitUntil( () => { - AutomationElement? cancel = FindById(MainWindow, "btnCancel"); - - // Gone means the window hid it, which it does only when a run ends - but - // that has to come from the status, not from the button's absence. - if (cancel is null) - return ConversionHasFinished(); - - // A refusal throws out of here into the retry loop, which reacquires the - // button on the next attempt. If the run really has ended by then, the - // branch above sees the status say so. - Invoke(cancel); - cancelled = true; - return true; + // Once a click may be in flight, stop pressing the button and just watch. + if (!clickMayHaveLanded) + { + AutomationElement? cancel = FindById(MainWindow, "btnCancel"); + + // Gone means the window hid it, which it does only when a run ends - + // but that has to come from the status, not the button's absence. + if (cancel is not null) + { + try + { + Invoke(cancel); + clickMayHaveLanded = true; + } + catch (ElementNotEnabledException) + { + // Refused outright, so nothing was delivered. Try again. + return false; + } + catch (Exception ex) when ( + ex is ElementNotAvailableException or COMException) + { + clickMayHaveLanded = true; + } + } + } + + return ConversionHasFinished(); }, - "Cancel was never accepted, and the run never reported that it had stopped."); - - if (!cancelled) - { - throw new GuiDriverException( - "The conversion finished before cancellation could be exercised."); - } + "The run never reported a final status after cancellation was requested."); } /// Requires the cancellation request to produce an interrupted run. @@ -599,7 +612,7 @@ private void SelectCombo( { AutomationElement combo = RequireById(root, automationId); - if (SelectedName(combo).Equals(value, StringComparison.OrdinalIgnoreCase)) + if (string.Equals(SelectedName(combo), value, StringComparison.OrdinalIgnoreCase)) return; if (!combo.TryGetCurrentPattern(ValuePattern.Pattern, out object? rawValue)) @@ -616,7 +629,7 @@ private void SelectCombo( setter.SetValue(value); WaitUntil( - () => SelectedName(combo).Equals(value, StringComparison.OrdinalIgnoreCase), + () => string.Equals(SelectedName(combo), value, StringComparison.OrdinalIgnoreCase), $"'{value}' was not selected in '{automationId}'."); } @@ -628,11 +641,14 @@ private static string SelectedName(AutomationElement combo) ((SelectionPattern)rawSelection).Current.GetSelection(); if (selected.Length > 0) - return selected[0].Current.Name; + return selected[0].Current.Name ?? string.Empty; } + // A provider may hand back null for either of these. Absorbing it here means + // callers can compare the result without guarding, and an unreadable selection + // fails their assertion rather than their null check. if (combo.TryGetCurrentPattern(ValuePattern.Pattern, out object? rawValue)) - return ((ValuePattern)rawValue).Current.Value; + return ((ValuePattern)rawValue).Current.Value ?? string.Empty; return string.Empty; } diff --git a/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs b/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs index 3130bd1..60e65a3 100644 --- a/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs +++ b/sources/EncodingChecker.GuiSmoke/SmokeSuite.cs @@ -184,12 +184,12 @@ private void PhaseA(PhaseContext phase) // over a control label's casing. gui.SetTargetEncoding("us-ascii"); string changed = gui.TargetEncoding(); - Check(changed.Equals("us-ascii", StringComparison.OrdinalIgnoreCase), + Check(string.Equals(changed, "us-ascii", StringComparison.OrdinalIgnoreCase), $"The target encoding did not change: {changed}"); gui.SetTargetEncoding("utf-8"); string restored = gui.TargetEncoding(); - Check(restored.Equals("utf-8", StringComparison.OrdinalIgnoreCase), + Check(string.Equals(restored, "utf-8", StringComparison.OrdinalIgnoreCase), $"The target encoding did not change back: {restored}"); AssertSameFiles(before, Snapshot(directory)); From 3c700e4a735d56d1eceead1e6ef94e1cc5557831 Mon Sep 17 00:00:00 2001 From: amrali-eg <32075105+amrali-eg@users.noreply.github.com> Date: Thu, 10 Sep 2026 01:12:15 +0300 Subject: [PATCH 6/6] Retry only the click that certainly delivered nothing Two gaps in how RequestCancel classified a failed press. A refusal was caught and answered "not ready", which is a clean answer, and the shared loop clears its retained error on one of those - so the diagnostic was erased again, the same way the earlier local catch erased it. The refusal is now left to that loop, which retries it and keeps it as the cause a timeout can name. Every other automation failure from the press is now treated as an uncertain outcome, not only ElementNotAvailableException and COMException. A plain InvalidOperationException escaped to the shared loop, which retried the whole predicate and pressed a button whose outcome was unknown: instrumented, that shape presses Cancel 35-38 times where one press is correct. It now presses once and waits for the status. Controls on the corrected shape: an uncertain press that never landed presses once and the phase fails with "Cancellation was not exercised", five refusals followed by acceptance still cancel and interrupt the run, and a press that lands and then throws still passes. EC is unchanged. Co-Authored-By: Claude Opus 5 --- .../EncodingChecker.GuiSmoke/EcGuiDriver.cs | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs b/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs index b5d98de..b966f75 100644 --- a/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs +++ b/sources/EncodingChecker.GuiSmoke/EcGuiDriver.cs @@ -272,11 +272,13 @@ internal void ProceedThenCancel(AutomationElement review, Func writingHasB /// /// The two ways a click can fail mean different things. A refusal - the control /// reporting itself not-enabled - happens before anything is delivered, so trying - /// again is safe and right. An element that disappears mid-call, or a COM failure, - /// is what a click that *did* land looks like when the window hides the button in - /// response; it may equally have failed before delivering. Clicking again there - /// would be a fresh action rather than a retry, so the attempt stops and the status - /// is left to say what happened. + /// again is safe and right, and it is left to the shared retry loop, which repeats it + /// and keeps it as the cause a timeout would name. + /// + /// Any other automation failure might have followed a click that did land: an element + /// disappearing mid-call is what a successful cancel looks like when the window hides + /// the button in response. Clicking again there would be a fresh action rather than a + /// retry, so the attempt stops and the status is left to say what happened. /// private void RequestCancel() { @@ -299,13 +301,18 @@ private void RequestCancel() Invoke(cancel); clickMayHaveLanded = true; } - catch (ElementNotEnabledException) - { - // Refused outright, so nothing was delivered. Try again. - return false; - } + // A refusal is the one failure that certainly delivered nothing, + // and it is deliberately not caught: the shared loop retries it + // and keeps it, so a wait that expires on repeated refusals can + // name them. Answering "not ready" here would clear that cause. + // + // Every other automation failure might have followed a click that + // landed, so the attempt stops and the status is left to say. catch (Exception ex) when ( - ex is ElementNotAvailableException or COMException) + ex is not ElementNotEnabledException && + ex is ElementNotAvailableException + or COMException + or InvalidOperationException) { clickMayHaveLanded = true; }