Skip to content

Fix printing of log messages on test failures #147

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

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

blaugold
Copy link
Contributor

@blaugold blaugold commented Oct 2, 2023

The global singleton logger that was used before associated all log messages with the first test that accessed it.

The issue was that calling onRecord.listen() captured the Zone of the first access of logger and thus of the first test that uses logger. printOnFailure in turn uses Zones to associate messages with a specific test.

@coveralls
Copy link

coveralls commented Oct 2, 2023

Coverage Status

coverage: 98.172%. remained the same when pulling 48af904 on blaugold:fix-test-logging into 22500ea on dart-lang:main.

@dcharkes
Copy link
Collaborator

dcharkes commented Oct 3, 2023

Thanks @blaugold !

printOnFailure(record.message);
});
Logger get logger => _logger ??= () {
// We lazily create a new logger for each test so that the messages
Copy link
Collaborator

Choose a reason for hiding this comment

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

Avoid "we".

@auto-submit
Copy link

auto-submit bot commented Oct 3, 2023

auto label is removed for dart-lang/native/147, due to This PR has not met approval requirements for merging. You are not a member of dart-team and need 1 more review(s) in order to merge this PR.

  • Merge guidelines: You need at least one approved review if you are already part of flutter-hackers or two member reviews if you are not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@dcharkes dcharkes merged commit 23d46c3 into dart-lang:main Oct 3, 2023
HosseinYousefi pushed a commit that referenced this pull request Nov 16, 2023
* Move summarizer invocation into summary.dart for better testability.
* Support reading source JARs (#208)
* Fix an issue where the summarizer failed when providing only classes and no source (#147)
HosseinYousefi pushed a commit that referenced this pull request Nov 16, 2023
* Move summarizer invocation into summary.dart for better testability.
* Support reading source JARs (#208)
* Fix an issue where the summarizer failed when providing only classes and no source (#147)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants