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

fix(core): import mjs path correctly when running on windows #2662

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

thrandale
Copy link
Contributor

This fixes issue #2661

@CLAassistant
Copy link

CLAassistant commented Apr 22, 2024

CLA assistant check
All committers have signed the CLA.

@hassy
Copy link
Member

hassy commented Apr 22, 2024

thanks @thrandale! Is the issue due to a missing relative path? ie. in your example ./artilleryRunner.mjs should work as expected (if the MJS file is in the same directory as the YAML script). There's a test for loading ESM here, would you mind updating that please? https://github.com/artilleryio/artillery/blob/242a7964934851bf77e53eebf44e6312de723663/packages/artillery/test/cli/async-hooks-esm.test.js

@thrandale
Copy link
Contributor Author

thrandale commented Apr 22, 2024

@hassy Even using a relative path, it still fails with the same error. When it does the path.resolve(), that resolve either an absolute or relative path to an absolute path. It is odd that it is not failing in the CI. I believe this is only an issue on Windows. Does the CI run the windows tests?

@ggecy
Copy link

ggecy commented Jul 18, 2024

@hassy Even using a relative path, it still fails with the same error. When it does the path.resolve(), that resolve either an absolute or relative path to an absolute path. It is odd that it is not failing in the CI. I believe this is only an issue on Windows. Does the CI run the windows tests?

I can confirm this - we have this problem only on windows, I can run it on macos without error. This is making the artillery completely unusable on windows if we have typescript configured to compile playwright tests to esm modules.

@bernardobridge
Copy link
Contributor

bernardobridge commented Jul 29, 2024

Hi @thrandale 👋 ! Thanks for the contribution. We have now added windows tests to CI, so we could confirm fixes like this.

I've opened #3293 with your fix and ran our full suite against it, and it seems to work 👍 . If you want to rebase this branch with main (as it contains the windows tests), I can close my PR and merge this one once the test suite runs here too. If you do that in the next couple of days (as we aim to release on Monday), I'll use your PR. Otherwise I'll use my PR and reference you in the release anyway.

Thanks again!

@thrandale
Copy link
Contributor Author

Thanks @bernardobridge. I have rebased my branch off main.

@bernardobridge
Copy link
Contributor

Hi @thrandale, thanks for that. I'm merging this as the tests as passing (they appear red, but it's a bug that happens when running against contributor PR's).

Thanks again for the fix! It'll be out in 2.0.19 next week.

@bernardobridge bernardobridge changed the title fix Error [ERR_UNSUPPORTED_ESM_URL_SCHEME] when using a artillery runner file with a .mjs extension fix(core): import mjs path correctly when running on windows Jul 29, 2024
@bernardobridge bernardobridge merged commit ca69167 into artilleryio:main Jul 29, 2024
56 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants