Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

return png2src error to caller #787

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

peppergrayxyz
Copy link
Contributor

fix #786

lang: "rust",
output: "-",
});
expect(console.error).toHaveBeenCalledWith("Error processing /invalid/path: ENOENT: no such file or directory, open '/invalid/path'");
})).toThrowError(/Error processing \/invalid\/path: ENOENT: no such file or directory, open '\/invalid\/path'/);
Copy link
Contributor

Choose a reason for hiding this comment

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

@peppergrayxyz I think it could be worth asserting the process exits with code error 1 with something like

const mockExit = jest.spyOn(process, 'exit').mockImplementation(() => {});
// expect with runAll() call here then
expect(mockExit).toHaveBeenCalledWith(1);

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to fit it into the existing logic and run in png2src.js does not return a value either, but only throws an exception, so I though runAll should behave the same way. Also, the other modules don't return a value either, but throw exceptions. So I think an exception is the most consistent thing to do here.

If we want to check for return 1 I think we should do it in the test for cli.js and then also for all such cases, but there are no tests for that file (yet).

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, I don't know this tool yet, you are perfectly right. Thanks for your detailed answer.

@aduros
Copy link
Owner

aduros commented Dec 13, 2024

Thanks!

@aduros aduros merged commit 363d122 into aduros:main Dec 13, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

png2src return value does not report errors to the caller
3 participants