Improve user-facing messages #3175
Conversation
| @@ -213,9 +213,6 @@ public string LoadTests() | |||
| /// <returns>The XML result of exploring the tests</returns> | |||
| public string ExploreTests(string filter) | |||
| { | |||
| if (Runner.LoadedTest == null) | |||
| throw new InvalidOperationException("The Explore method was called but no test has been loaded"); | |||
|
|
|||
| return Runner.ExploreTests(TestFilter.FromXml(filter)).ToXml(true).OuterXml; | |||
There was a problem hiding this comment.
This check is literally the first thing Runner.ExploreTests does.
| throw new InvalidOperationException("The Explore method was called but no test has been loaded"); | ||
|
|
||
| handler.RaiseCallbackEvent(Runner.ExploreTests(TestFilter.FromXml(filter)).ToXml(true).OuterXml); | ||
| handler.RaiseCallbackEvent(ExploreTests(filter)); |
There was a problem hiding this comment.
Identical implementation within a project should be shared.
jnm2
force-pushed
the
improve_user_facing_messages
branch
from
86acc5d to
780a7c4
Compare
jnm2
force-pushed
the
improve_user_facing_messages
branch
from
780a7c4 to
f2d4a96
Compare
|
@nunit/framework-team Fixed the test failures. Ready for review. |
There was a problem hiding this comment.
This is pretty wide-ranging but looks good to me.
It does bring up for me a related question: Should these all be user-facing messages? At least a few of them can only arise due to a programming error in the console, for example, the message about calling explore before the test has been loaded. Maybe those type of messages should simply be eliminated or prefixed with something like "Internal Error:"
However, that could be done separately if we want to change it.
Ran out of time, but small PRs are fun PRs. I didn't get to the motivating example I ran into today, so I'll be back with more soon.
We should try to use full sentences in all messages to the user, including obsoletion and exception messages, message boxes, console messages, and XML documentation.