Skip to content

Conversation

@harishsk
Copy link
Contributor

@harishsk harishsk commented Jan 26, 2020

Currently when running tests the channel output doesn't appear in the test logs. This is a draft change to enable that functionality in order to get better logging to debug tests better.

There are two changes here.
The first is a simple adds a handler for MLContext's Log event and logs the message to test output. The second adds a TextWriter derived class to ConsoleEnvironment that makes ConsoleEnvironment also log to the test console if the code is running a test environment.

I am putting this PR up for review for two purposes.

The first is for general verification of the code and concept. And while the code reviews are in progress I am also using this to try and debug the failure in BinaryClassifierSymSgdTest in the CI builds.

@harishsk harishsk requested a review from a team as a code owner January 26, 2020 01:00
@harishsk harishsk requested a review from a team as a code owner January 28, 2020 05:21
@justinormont
Copy link
Contributor

Related issues on having an output verbosity level besides zero & firehose:

@harishsk
Copy link
Contributor Author

I understand the issue about the all or nothing logs. It would be good to add additional params to control the verbosity but that will impact the public API. I want the full logs right now to debug test failures. Can we defer the issue of adding additional control params?

@justinormont
Copy link
Contributor

Can we defer the issue of adding additional control params?

Oh certainly. I was hoping/dreaming that both the unit test and user level logging could be solved in the same way.

@harishsk
Copy link
Contributor Author

I have added additional params to LoggingEventArgs

@sharwell
Copy link
Contributor

Can we not log to a file that gets attached as a build artifact?

@harishsk
Copy link
Contributor Author

I think it is better in the default test log. It gives more flexibility that if I am running a single test locally on the console, I can see the output on the console or redirect to a file as I see fit. And on the build server, it gets logged to a file anyway. It it still a file whether it is in the artifacts or the default log file.

Copy link
Contributor

@frank-dong-ms-zz frank-dong-ms-zz left a comment

Choose a reason for hiding this comment

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

:shipit:

@harishsk harishsk merged commit 609003e into dotnet:master Feb 1, 2020
@harishsk harishsk deleted the testLogger branch April 21, 2020 23:59
@ghost ghost locked as resolved and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants