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 ts-node version issue with webext sample project #1457

Merged
merged 2 commits into from
Aug 30, 2022

Conversation

rosahbruno
Copy link
Contributor

Description

I discovered that the webext tests were broken with newer versions of node. This is the same issue that we saw with the main glean package tests. I did not realize at first that this sample project also had its own tests and the same issues as the glean tests. The same fix that we did for glean proper can be applied here.

Notes on the other sample projects tests

  • web - This project does not have tests, so there is nothing to fix.
  • node - This project uses mocha directly for testing, it does not do any of the experimental ESM stuff like glean & webext
  • qt - The tests for this project has issues, but they are not related to the ts-node versions. I am logging a bug for those to be worked on separately.

Testing

  1. Run npm run test from inside of the samples/browser/webext folder with a node version greater than 10.5.x.
  2. Run the sample project and confirm you can see pings in the debug viewer.

Pull Request checklist

  • Quality: Make sure this PR builds and runs cleanly.
    • Inside the glean/ folder, run:
      • npm run test Runs all tests
      • npm run lint Runs all linters
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry to CHANGELOG.md or an explanation of why it does not need one
  • Documentation: This PR includes documentation changes, an explanation of why it does not need that or a follow-up bug has been filed to do that work

@rosahbruno rosahbruno marked this pull request as ready for review August 29, 2022 18:03
@auto-assign auto-assign bot requested a review from chutten August 29, 2022 18:03
Copy link
Contributor

@chutten chutten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rosahbruno rosahbruno merged commit 4dafaf1 into mozilla:main Aug 30, 2022
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.

2 participants