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

Node integration test enviroment #5241

Merged
merged 4 commits into from
Jan 11, 2023
Merged

Conversation

kraenhansen
Copy link
Member

What, How & Why?

This fixes up the Node.js integration test environment, making it ESM friendly in the process.

I had to temporarily skip the Node.js specific "analytics" tests, as they need to be migrated off the non-esm Node.js APIs.

☑️ ToDos

  • 📝 Changelog entry
  • 📝 Compatibility label is updated or copied from previous entry
  • 📝 Update COMPATIBILITY.md
  • 🚦 Tests
  • 🔀 Executed flexible sync tests locally if modifying flexible sync
  • 📦 Updated internal package version in consuming package.jsons (if updating internal packages)
  • 📱 Check the React Native/other sample apps work if necessary
  • 📝 Public documentation PR created or is not necessary
  • 💥 Breaking label has been applied or is not necessary

@kraenhansen kraenhansen self-assigned this Jan 11, 2023
@cla-bot cla-bot bot added the cla: yes label Jan 11, 2023
@kraenhansen kraenhansen changed the base branch from master to bindgen January 11, 2023 11:17
Copy link
Contributor

@takameyer takameyer left a comment

Choose a reason for hiding this comment

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

Just a few small things, but nothing holding this back. LGTM

require("@realm/integration-tests");
// Add the integration test suite
await import("@realm/integration-tests");
console.log("integration tests loaded");
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed console.logs here and in the updated mocha-client. Do we want to keep those in, or were they just for testing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yeah - good catch. They need to go 🙂

import "./utils/import-app.test";
import "./utils/chai-plugin.test";
import "./utils/import-app.test.ts";
import "./utils/chai-plugin.test.ts";
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to detect these extensions automatically?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I played around with it. It think it gets confused about the .test part of the extension 😕

Copy link
Contributor

Choose a reason for hiding this comment

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

I was getting this issue as well when I was getting the react-native tests to run again, but in a different section. Wasn't really able to resolve it. Very odd.

@kraenhansen kraenhansen merged commit e354836 into bindgen Jan 11, 2023
@kraenhansen kraenhansen deleted the kh/bindgen/node-test-env branch January 11, 2023 15:40
kraenhansen added a commit that referenced this pull request Mar 1, 2023
* Adding mocha-reporter and node testenv to NPM WS

* Making the Node tests and environment ESM friendly

* Upgrading to the latest Mocha Remote

* Cleaned up some console.logs
kraenhansen added a commit that referenced this pull request Mar 3, 2023
* Adding mocha-reporter and node testenv to NPM WS

* Making the Node tests and environment ESM friendly

* Upgrading to the latest Mocha Remote

* Cleaned up some console.logs
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants