Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Investigate getting test specs to be able to use ESM import #16

Closed
mattcollier opened this issue Jun 17, 2020 · 4 comments
Closed

Investigate getting test specs to be able to use ESM import #16

mattcollier opened this issue Jun 17, 2020 · 4 comments
Labels
enhancement Priority 2 Important but not critical

Comments

@mattcollier
Copy link
Contributor

I spent some time looking into this and tried some things at the test suite level for a bedrock-module.

https://stackoverflow.com/a/60522428

According to that, we might just be able to call mocha -r esm here

@davidlehn
Copy link
Member

That method has been working for standalone testing. Maybe a related issue is getting coverage working with nyc, mocha, and esm code. nyc also has a way to require esm, but I've had issues with it sometimes working, sometimes not, in the same repo, and no idea what changed. There may be some general pattern that helps with mocha and nyc.

@JSAssassin
Copy link

JSAssassin commented Dec 2, 2021

Also experiencing issues with coverage with nyc not working with bedrock-foo repos that are using esm module. It works sometimes and most of the times it doesn't work, and there are no errors.

One of the examples would be https://github.com/digitalbazaar/bedrock-meter. When running the coverage script, it gives the following summary and in the report we can only see the index.js file.

Statements   : 100% ( 2/2 )
Branches     : 100% ( 0/0 )
Functions    : 100% ( 0/0 )
Lines        : 100% ( 2/2 )

Screen Shot 2021-12-02 at 2 02 34 PM

Tried adding the nyc --require esm flag to the coverage script in bedrock-meter test package.json but that didn't work either and gave the same result as above.

Some of the repos that might be impacted by this are:

@JSAssassin JSAssassin added the Priority 1 Critical label Dec 2, 2021
@mattcollier
Copy link
Contributor Author

I found this issue which gave me the idea that c8 might be a drop-in replacement for nyc

istanbuljs/nyc#1353

https://github.com/bcoe/c8

Which appears to be the case:

This PR shows the necessary mods:
digitalbazaar/bedrock-meter#4

https://app.codecov.io/gh/digitalbazaar/bedrock-meter/commits?page=1

@dlongley dlongley added Priority 2 Important but not critical and removed Priority 1 Critical labels Mar 17, 2022
@dlongley
Copy link
Member

This has been fixed, we can use ESM import now, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Priority 2 Important but not critical
Projects
None yet
Development

No branches or pull requests

5 participants