Skip to content

feat: force the returned value in test.catch() and test.catchOrNull() - #1910

Merged
HT154 merged 2 commits into
apple:mainfrom
AAGAM17:feat/test-catch-forces-returned-value
Oct 16, 2026
Merged

HT154 merged 2 commits into
apple:mainfrom
AAGAM17:feat/test-catch-forces-returned-value

Conversation

@AAGAM17

@AAGAM17 AAGAM17 commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Fixes #1899.

test.catch() and test.catchOrNull() applied the lambda but did not force its result, so a lazily failing value such as new Listing { 1 } as Listing<String> did not throw inside the call.

Both now force the returned value (VmValue.force(result, false), the same forcing rendering does), so errors in its members are caught. The behavior is documented on both functions.

In the as3 snippet test, two catchOrNull(...) == null results change from true to false, because those values now throw when forced. I also added the example from the issue.

I could not run the Gradle build locally (disk space), so CI will be the first full run of the language snippet tests.

@bioball bioball left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR! This generally looks good, just some comments.

Comment thread pkl-core/src/test/files/LanguageSnippetTests/input/basic/as3.pkl Outdated
Comment thread stdlib/test.pkl Outdated
Move the test to api/testModule.pkl and reword the stdlib docs.
@AAGAM17
AAGAM17 requested a review from bioball October 16, 2026 18:39

@bioball bioball left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thank you!

Looking for one more maintainer to approve this PR before merging.

@HT154 HT154 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@HT154
HT154 merged commit e18d962 into apple:main Oct 16, 2026
25 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.

Consider making pkl.test#catch() implicitly force its arg's return value

3 participants