Add more tests to ensure proper handling of undefined#131
Add more tests to ensure proper handling of undefined#131cjbarth wants to merge 1 commit intogoto100:masterfrom
Conversation
|
I'm sorry for leaving this project unattended for so long. The flurry of incoming requests in July got overwhelming and drew time away from things that I needed to be spending time on, and I developed a mental block against spending any time on this. Thank you for proposing these changes. Today, I've added some additional error handling to throw somewhat helpful errors when:
As of these changes, the first test you've proposed is no longer relevant because an unspecified expression is now an error condition and I have added a test for that. This also makes most of the .d.ts changes in this PR moot, though I think the renaming of Your second and third tests, I would say, "incorporate too many variables" because in both of them, you are passing an With regard to the second test, hopefully my new error handling and two new tests address this, in a slightly more thorough manner. For the third test, I think it's painting a bit of an incomplete picture. Not passing a resolver can result in an empty result set, but only under certain circumstances. And as I say above, the situation in your test is muddied by the lack of an XPath expression. But if you can identify a situation where omitting the resolver results in an outcome that one would not reasonably expect, I may be able to find a way to improve that situation. Thanks again. |
When using this code over at
node-saml, I found that we were sendingundefinedornullon occasion. I've added some tests, but changed no behavior, to validate what was actually happening internally. I then updated the TypeScript types to match the tests.This PR focuses on minimal changes for correctness involving these new tests, so I didn't address any of the broader feedback on types.