Skip to content

Better error messages on close#23

Open
jackkleeman wants to merge 1 commit intomainfrom
close
Open

Better error messages on close#23
jackkleeman wants to merge 1 commit intomainfrom
close

Conversation

@jackkleeman
Copy link
Contributor

This brings more in line with the go impl; exec manager tracks if it has exited and won't try to send stuff if it has, evaluators can close themselves at which point they will throw sensible errors if you try to use them.

This brings more in line with the go impl; exec manager tracks if it has exited and won't try to send stuff if it has, evaluators can close themselves at which point they will throw sensible errors if you try to use them.
@jackkleeman jackkleeman requested a review from jasongwartz March 17, 2024 11:08
@jasongwartz
Copy link
Contributor

Looks like we've got tests hanging:

 PASS e2e/only_primitives/primitives.test.ts
  E2E of config with only primitive values
    ✓ can generate TypeScript sources and load valid values (291 ms)
    ✓ can generate TypeScript sources but error on evaluating invalid values: missing a required property (225 ms)
    ✓ can generate TypeScript sources but error on evaluating invalid values: property is of the wrong type (213 ms)
    ✓ can generate TypeScript sources but error on evaluating invalid values: property fails validation constraint (215 ms)
Test Suites: 1 passed, 1 total
Tests:       4 passed, 4 total
Snapshots:   0 total
Time:        2.961 s
Ran all test suites.
Jest did not exit one second after the test run has completed.
'This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.

The latter three are test cases where Pkl itself will error and exit non-zero. The fact that they're succeeding means an error must be being thrown (as expected), but it looks like this change is causing something to hang open.

@jackkleeman
Copy link
Contributor Author

ah, damn. will work on it. nice tests!

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