fix: Refactor for bailing out wait time

[dprevost-LMI]

As pointed out by Greptile in Bug B here, implementing a bail-out mechanism to abort earlier in cases where we know we will fail and do not need to exhaust all the wait time.
By refactoring waitUntil to receive the full context, we can enhance its behaviour while still returning the appropriate fetched actual and subject (like an awaited element).

This also streamlines the code instead of using a local variable to capture modified actual & elements (subject)

[greptile-apps]

Greptile Summary

Refactors matcher polling to retain the complete comparison context and terminate immediately when further retries cannot change the outcome.

• Changes waitUntil conditions and return values from booleans to structured results containing success, subject, actual value, and an optional abort signal.
• Adds abort signals for empty subjects and incompatible element/value cardinalities.
• Updates element, browser, elements-array, and mock matchers to consume the structured result directly.
• Renames comparison result fields from result/value to success/actual and updates related tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported abort path now returns directly, preserves the comparison context, and is covered for both assertion polarities with a single condition invocation.

Important Files Changed

Filename	Overview
src/util/waitUntil.ts	Returns structured condition context and now exits directly on abort without allowing the retry handler to intercept it.
src/util/executeCommand.ts	Standardizes comparison result fields and marks terminal empty-subject or cardinality-mismatch outcomes for early bailout.
src/utils.ts	Adapts shared comparison and boolean-matcher helpers to the structured result contract.
test/util/waitUntil.test.ts	Covers immediate abort behavior for both assertion polarities and verifies that the condition executes only once.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Matcher invokes waitUntil] --> B[Evaluate condition]
    B --> C[Structured strategy result]
    C --> D{Assertion polarity satisfied?}
    D -->|Yes| E[Return subject, actual, and success]
    D -->|No| F{Abort requested?}
    F -->|Yes| G[Return immediately without retry]
    F -->|No| H[Sleep and retry]
    H --> B

Loading

_{Reviews (9): Last reviewed commit: "code review" | Re-trigger Greptile}

[dprevost-LMI]

@greptileai, double-check deeply if modification can brings side effect or regressions

[dprevost-LMI]

@greptileai, to align both CompareResult & StrategyResult for a more seamless passthrough of the data, which structure would you recommend for both?

[greptile-apps]

Looking at the current type definitions from the diff:

export type CompareResult<T> = { result: boolean; value: T }
export type StrategyResult<T, E = ...> = { subject: E; success: boolean; actual: T | undefined; abort?: boolean; }
export type StrategyMaybeArrayResult<T, E = ...> = { subject: E; success: boolean; actual: MaybeArray<T> | undefined; abort?: boolean; }

The friction points are:

1. Field name mismatch: CompareResult uses result/value while StrategyResult uses success/actual
2. Unnecessary split: StrategyMaybeArrayResult vs StrategyResult differ only in actual: MaybeArray<T> vs actual: T

My recommendation:

Unify CompareResult field names with StrategyResult:

export type CompareResult<T> = { success: boolean; actual: T }

This lets CompareResult read as a structural subset of StrategyResult (minus subject/abort), making the conceptual mapping obvious and eliminating the rename boilerplate in executeCommand.ts where compareResult.result → success and compareResult.value → actual.

Collapse StrategyMaybeArrayResult into StrategyResult:

export type StrategyResult<T, E = WebdriverIO.Element | WebdriverIO.ElementArray | WebdriverIO.Element[] | WebdriverIO.Browser | unknown> = {
    subject: E;
    success: boolean;
    actual: T | undefined;
    abort?: boolean;
}

Callers that need array actuals use StrategyResult<MaybeArray<SomeType>> explicitly — e.g. legacyMultipleElementResultsStrategy becomes Promise<StrategyResult<MaybeArray<Actual>>>. This removes the parallel type while keeping the distinction in the type parameter where it's visible.

The end result: CompareResult<T> → StrategyResult<T> is a clean widening (add subject + optional abort), and MaybeArray vs scalar is expressed at the call site rather than through a separate interface.

[dprevost-LMI]

@greptileai, do you see any further changes or simplifications we should make to CompareResult & StrategyResult to streamline the code?

[greptile-apps]

A few concrete simplifications stand out:

1. StrategyResult should extend CompareResult instead of duplicating fields

Both share success: boolean and actual. StrategyResult is just CompareResult with subject and abort bolted on:

export type StrategyResult<T, E = unknown> = CompareResult<T | undefined> & {
    subject: E;
    abort?: boolean;
}

This removes the duplication and makes the relationship between the two types explicit.

2. The default type for E collapses to unknown

// current — the union is vacuous because `unknown` subsumes everything
E = WebdriverIO.Element | WebdriverIO.ElementArray | WebdriverIO.Element[] | WebdriverIO.Browser | unknown

Since unknown is the top type, the rest of the union adds zero type safety. Either drop it to just unknown, or (if the intent is documentation) move those specific types to a named alias:

export type SubjectType = WebdriverIO.Element | WebdriverIO.ElementArray | WebdriverIO.Element[] | WebdriverIO.Browser
export type StrategyResult<T, E = SubjectType | unknown> = ...

3. Avoid the mutation in waitUntil

In waitUntil.ts, there's a direct mutation before returning:

result.success = !isNot   // mutates the caller's object
return result

Prefer a spread to make the return value a new object:

return { ...result, success: !isNot }

4. CompareResult<T> vs CompareResult<T | undefined>

CompareResult<T> has actual: T (non-optional), but StrategyResult has actual: T | undefined. If you go with the intersection approach above, CompareResult itself stays clean and the | undefined is only introduced at the StrategyResult level, which is the right boundary since strategies can have no element to compare against.

The highest-value change here is #1 — it removes structural duplication and means any future field added to CompareResult automatically flows into StrategyResult.

[dprevost-LMI]

@greptileai, do you see any further changes or simplifications we should make to CompareResult & StrategyResult to streamline the code? You can skip 2. The default type for E collapses to unknown

[dprevost-LMI]

@greptileai, do you see any further changes or simplifications?