Hi @JSteunou! Thanks for your contribution! Can you please add tests to cover those line changes to keep code coverage at 100%?

Sure but I might need some help for that

@JSteunou Sure! change one of these helpers to ESM and run npm test to make sure the code coverage goes to 100% https://github.com/hapijs/vision/tree/master/test/templates/valid/helpers

I was torn between adding a new test just for this feature or changing one of the existing helpers to ESM but it's really not worth a new test I think, unless someone else who works on this module thinks otherwise

How will you hit full code coverage without a new test (or two)?

All the helpers are being used and are CommonJS modules -- the new changes should be covered if one of them is changed to ESM

Ah, I see what you mean. There are a few cases here to hit,

const helper = required[name] || required.default || required;

Oh yeah! whoops =P
@JSteunou In order to hit 100% code coverage can you please add a dedicated test for ESM helpers?
You can use this test as a starting point to copy / paste / modify
https://github.com/hapijs/vision/blob/master/test/index.js#L589-L605

The helpers can do whatever you like, they can even just be copies of 'long' and 'uppercase', but exposed as ESM modules.

We'll need to cover the cases where one is exported as default, and the other is a named export

Checklist:

• 2 new ESM helpers in https://github.com/hapijs/vision/tree/master/test/templates/valid/helpers
  • One exports as default
  • One is a named export
• New test to cover both of these similar to https://github.com/hapijs/vision/blob/master/test/index.js#L589-L605

Sure, thanks for the hints, I'll try this asap

Ok I had to simulate ESModule behavior with CJS exports because I couldn't update the all test suite, but that should be enough :)

Hmmm I'm asking myself why are these weird global leaks happening all of a sudden causing tests to fail? The following leaks were detected:__core-js_shared__

This line in hapi-react-views seems to mitigate this issue by ignoring it on the lab test call, I don't know why it's happening rn tho.. https://github.com/jedireza/hapi-react-views/blob/master/package.json#L7

New node 8.10 or related to what I added?

The only other issue I have is the tests aren't using ESM -- but I'm still considering merging it

If you have a solution to use true ESM inside test I will take it ;)

After poking around, ESM in a test without babel looks like a nightmare -- no need to add anything more.

Thank you so much @JSteunou for your contribution!!

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.