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

Fixed support for namespaced modules. Closes #199 #209

Merged
merged 2 commits into from
Jun 22, 2023

Conversation

jan-molak
Copy link
Contributor

@jan-molak jan-molak commented Jun 20, 2023

Serenity/JS provides a Jasmine adapter published as a namespaced module @serenity-js/jasmine.

This change adds support for Jasmine loader.js to load namespaced modules and closes #199.

@jan-molak
Copy link
Contributor Author

@sgravrock looks like Windows tests use OS-dependent paths. Let me know how you'd like to proceed, please.

@sgravrock
Copy link
Member

I can think of a few options:

  • Modify the failing tests to use a regex that matches the desired URL with or without a drive letter
  • Modify the failing tests to figure out the drive letter that will be resolved and build the right expected URL, if on Windows (probably not worth the complexity)
  • Go back to converting the absolute path to a URL by hand instead of url.pathToFileURL (would be unfortunate since url.pathToFileURL is a nice improvement, but it doesn't necessarily have to be in this PR)

@jan-molak
Copy link
Contributor Author

Thanks for the feedback, @sgravrock! I played around with the regex, but it seems like repeating url.pathToFileURL in the test as well might be easier to read:

expect(importShim).toHaveBeenCalledWith(url.pathToFileURL('/the/path/to/the/module').toString());

If you prefer a regex please let me know and I can update the PR.

@sgravrock sgravrock merged commit 097bb29 into jasmine:main Jun 22, 2023
@sgravrock
Copy link
Member

Thanks for the PR.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--reporter does not work with namespaced modules
2 participants