Skip to content

Update tests and docs for waitForCondition#241

Merged
zhongkechen merged 1 commit intomainfrom
waitforcondition-tests-docs-update
Mar 20, 2026
Merged

Update tests and docs for waitForCondition#241
zhongkechen merged 1 commit intomainfrom
waitforcondition-tests-docs-update

Conversation

@nvasiu
Copy link
Contributor

@nvasiu nvasiu commented Mar 19, 2026

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Issue Link, if available

#90

Description

  • Added new unit tests to cover previously uncovered branches in waitForConditionOperation (used the new coverage report to find uncovered branches).

    • Coverage is not 100%, but the remaining uncovered branches are difficult to create unit tests for, and are already effectively covered by integration tests.
  • Updated the testCheckFunctionError waitForCondition integration test, to verify that an exception from the user provided check function is propagated properly.

  • Added new unit test for when deserializing a waitForCondition checkpoint fails, which verifies a SerDesException is thrown and propagated.

  • Updated waitForConditionExample to demonstrate a more realistic scenario.

  • WaitForConditionResult (and it's test) was moved to the model/ folder.

  • Simplified comments / removed unnecessary comments in general.

  • Updated existing docs / added new doc for waitForCondition

Demo/Screenshots

N/A

Checklist

  • I have filled out every section of the PR template
  • I have thoroughly tested this change

Testing

Unit Tests

Have unit tests been written for these changes?

Yes

Integration Tests

Have integration tests been written for these changes?

Yes

Examples

Has a new example been added for the change? (if applicable)

Yes

@nvasiu nvasiu requested a review from a team March 19, 2026 23:45
? WaitForConditionResult.stopPolling(latest)
: WaitForConditionResult.continuePolling(latest);
},
"PENDING");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if initialState is still useful to users. Looks like users can always directly pass their initial state into the lambda without a parameter.


private WaitStrategies() {
// Utility class - prevent instantiation
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we remove this?

├── CallbackException - General callback exception
│ ├── CallbackFailedException - External system sent an error response to the callback. Handle the error or propagate failure
│ └── CallbackTimeoutException - Callback exceeded its timeout duration. Handle the error or propagate the failure
├── WaitForConditionException - waitForCondition exceeded max polling attempts or failed. Catch to implement fallback logic.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with a subclass WaitForConditionFailedException?

@zhongkechen zhongkechen merged commit 3dc0c2a into main Mar 20, 2026
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants