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

Cleaning up console output from tests #2548

Merged
merged 12 commits into from
Oct 4, 2019
Merged

Conversation

kraenhansen
Copy link
Member

What, How & Why?

I was getting back to Realm JS and got annoyed that the output from the tests are bloated with console.logs and output from the Realm JS client itself. It turned out the Realm.Sync loggers got reset before each test executed (as Realm.clearTestState was called): To ensure the logger is restored, this PR monkey-patches Realm.clearTestState to reset the logger and log level to appropriate levels.

☑️ ToDos

  • 📝 Changelog entry
  • 📝 Compatibility label is updated or copied from previous entry
  • 🚦 Tests
  • 📝 Public documentation PR created or is not necessary
  • 💥 Breaking label has been applied or is not necessary

@kraenhansen kraenhansen self-assigned this Oct 3, 2019
Copy link
Contributor

@blagoev blagoev left a comment

Choose a reason for hiding this comment

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

looks ok, if it passes RN CI

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
tests/js/session-tests.js Outdated Show resolved Hide resolved
Co-Authored-By: Brian Munkholm <[email protected]>
@kraenhansen kraenhansen requested a review from bmunkholm October 3, 2019 13:11
Co-Authored-By: Brian Munkholm <[email protected]>
Copy link
Member

@tgoyne tgoyne left a comment

Choose a reason for hiding this comment

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

This has definitely been a consistent annoyance; both in how spammy it is by default and how awkward it was to actually make it log more when debugging things.

@kraenhansen
Copy link
Member Author

The failing tests looks like a brittle test harness to me:

  • The Node on Windows 8 seem to have a hard time finding the NodeJs executable
  • The two React Native example apps seem to have a hard time connecting to the metro / development server.

@blagoev blagoev self-requested a review October 4, 2019 06:55
Copy link
Contributor

@blagoev blagoev left a comment

Choose a reason for hiding this comment

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

its good

@blagoev
Copy link
Contributor

blagoev commented Oct 4, 2019

@kraenhansen yeah. lets merge it

@kraenhansen kraenhansen merged commit e9a958a into master Oct 4, 2019
@kraenhansen kraenhansen deleted the kh/test-output-cleanup branch October 4, 2019 07:19
kraenhansen pushed a commit that referenced this pull request Oct 4, 2019
* Clearing the default Jasmin reporters removes dots

* Removed setLogLevel calls from tests

* Removed excessive logging

* Removed setLogger call from notifier tests

* Setting up the Realm.Sync logger once

* Using the "debug" logger in session tests

* Adding missing dependencies

* Documenting changes

* Adding a release note

* Fixed issues reported by Codacy

* Apply suggestions from code review

Co-Authored-By: Brian Munkholm <[email protected]>

* Update tests/js/session-tests.js

Co-Authored-By: Brian Munkholm <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants