Powderhouse - Fix 2392 Error Message #2415
Conversation
Add additional error message to provide a good error message when the argument arity more than 1.
| .Errors | ||
| .Select(e => e.Message) | ||
| .Should() | ||
| .Contain(LocalizationResources.OptionArgumentsMaximumExceeded("-x", 3, 4)); |
There was a problem hiding this comment.
Can we use Single here as the state we are testing should be a single error.
There was a problem hiding this comment.
In this one we are not testing for a single error message. It is testing that the new error message is being used. In this case the setup of this test is setup to match the others in this test.
There was a problem hiding this comment.
We can table this because I think we have other things to focus on, I strongly think we are testing a single state with a limited focus and that limiting the confirmation sets us up for false positives.
The definition of single state with a limited focus can vary, which is of course why we disagree on this. This is a question I'd like the .NET libraries team to weigh in on, since they'll be working with this long term.
There was a problem hiding this comment.
There are a couple ways to approach this kind of test that would be aligned with what I've seen in the dotnet/runtime test practices.
- There should be tests that validate the expected errors are present
- There should be tests that validate no unexpected errors can pop up in the results
Those can be achieved together or separately. The approaches I've seen used in dotnet/runtime depend on how much the tests are adhering to DRY. If the tests follow DRY patterns and factor boilerplate away such that test scenarios can be reused across several tests, and it's therefore desired for the set of results to start to include more entries, then the test would just check that the expected result is one of the items in the set. We guard against unexpected entries in another way. But the default approach we take is for each test to validate the results match the entire set of expected results. Sequence equality is the preferred approach for that when feasible, but styles vary on this and you'll even see dotnet/runtime tests that perform asserts in a loop over the set.
One of the factors we keep in mind for dotnet/runtime is that there is overhead in discovering tests, and with the body of tests we have and the configurations we run them against, we favor multi-assert tests more than I personally would typically endorse--but extremely granular (pure) tests add time and money costs to every CI run.
Add additional error message to provide a good error message when the argument arity more than 1.
The previous error message was similar to "Option '-x' expects a single argument but 4 were provided."
After this fix the error message is similar to: "Option '-x' expects at most 3 arguments but 4 were provided."