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

Add default logger for tests #4730

Merged
merged 1 commit into from
Jul 15, 2022
Merged

Add default logger for tests #4730

merged 1 commit into from
Jul 15, 2022

Conversation

tomduncalf
Copy link
Contributor

@tomduncalf tomduncalf commented Jul 15, 2022

What, How & Why?

This adds a default logger for the integration tests, as we currently get no output on Android as it forwards stdout to /dev/null. The log is colourised to help with reading it.

image

☑️ ToDos

  • 📝 Changelog entry
  • 📝 Compatibility label is updated or copied from previous entry
  • 🚦 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

If this PR adds or changes public API's:

  • typescript definitions file is updated
  • jsdoc files updated
  • Chrome debug API is updated if API is available on React Native

@cla-bot cla-bot bot added the cla: yes label Jul 15, 2022
@tomduncalf tomduncalf force-pushed the td/add_default_test_logger branch from 69425e9 to 31db0a2 Compare July 15, 2022 14:25
@tomduncalf tomduncalf marked this pull request as ready for review July 15, 2022 14:27
@tomduncalf tomduncalf force-pushed the td/add_default_test_logger branch from 31db0a2 to fceda7a Compare July 15, 2022 14:32
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.

:shipit: The colors are great. Good idea!

@@ -30,6 +32,14 @@ export function importAppBefore(
} else {
this.app = await importApp(name, replacements);
Realm.App.Sync.setLogLevel(this.app, logLevel);
// Set a default logger as Android does not forward stdout
Realm.App.Sync.setLogger(this.app, (level, message) => {
const magentaDate = `\x1b[35m${new Date().toISOString().replace("T", " ").replace("Z", "")}`;
Copy link
Member

Choose a reason for hiding this comment

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

The full date timestamps feel a bit excessive to me. Perhaps an HH:MM:SS.ms would be enough?

Nit: I'd personally split this in two lines, I don't like debugging expressions inside of template strings (same goes for the line below).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea fair point maybe the data isn't that useful!

@tomduncalf tomduncalf merged commit def1ce6 into master Jul 15, 2022
@tomduncalf tomduncalf deleted the td/add_default_test_logger branch July 15, 2022 15:47
@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.

3 participants