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

WIP: add logging to test context #2443

Merged
merged 36 commits into from
Sep 11, 2021

Conversation

jschneidereit
Copy link
Member

Fixes #1992

@jschneidereit jschneidereit self-assigned this Sep 1, 2021
@jschneidereit jschneidereit force-pushed the jschneidereit/feature/1992-add-logging-to-testcontext branch 5 times, most recently from ce63b1f to ea14c7d Compare September 3, 2021 03:43
@jschneidereit jschneidereit force-pushed the jschneidereit/feature/1992-add-logging-to-testcontext branch from 298c9d2 to 18e5b95 Compare September 8, 2021 20:18
@jschneidereit jschneidereit force-pushed the jschneidereit/feature/1992-add-logging-to-testcontext branch from 18e5b95 to a880e2e Compare September 9, 2021 17:35
val logs = mutableListOf<LogEntry>()
val contextWithLogging = context.withCoroutineContext(TestContextLoggingCoroutineContextElement(logs))
try {
test(testCase, contextWithLogging)
Copy link
Member

Choose a reason for hiding this comment

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

You might want to wrap test(testCase, contextWithLogging) with a withContext call, because otherwise, downstream interceptors can change it without realizing.
I will fix this up.

}
}

typealias LogFn = suspend () -> Any
Copy link
Member

Choose a reason for hiding this comment

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

I'd remove suspend on logging

@sksamuel sksamuel merged commit cb8a17f into master Sep 11, 2021
@sksamuel sksamuel deleted the jschneidereit/feature/1992-add-logging-to-testcontext branch September 11, 2021 21:31
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.

TestContext should have a logging interface
2 participants