<p>I'm currently writing a parser for a small text file format, and in the process I found that testing for error values allowed for better coverage of the parser than just testing that correct inputs don't fail. For instance, usually errors have a string value that can be printed to the screen.</p>
<pre><code>var valid io.Reader
data, err := Parse(valid)
// error will be nil, which is fine
var invalid io.Reader
data, err := Parse(invalid)
// error holds a string value with info
if err != nil {
t.Errorf("invalid, got: %v:", err)
}
</code></pre>
<p>However, testing that specific inputs will result in specific errors is also important. I don't want an error from an invalid comment to show up as an error from an invalid integer literal. This is where I chose to create specific types e.g</p>
<pre><code>type commentError struct { error }
type integerError struct { error }
// parsing code
func Parse(r io.Reader) (Data, error) {
// ...
if v, err := strconv.Atoi(v); err != nil || v < 1 {
return nil, integerError{fmt.Errorf("bad int, %v", err)}
}
}
// testing code
if _, ok := err.(integerError); !ok {
t.Errorf("Wanted integer error, got %v", err)
}
</code></pre>
<p>Although this is unnecessary for smaller unit tests, it works well for testing large parts of the pipeline. However, I have no need for these custom error types during runtime, and they are only used for testing.</p>
<p>My questions are: Is it worth it to return custom error types specifically for tests? Is there a better way to go about testing error values other than creating custom types?</p>
<hr/>**评论：**<br/><br/>TheMerovius: <pre><blockquote>
<p>However, testing that specific inputs will result in specific errors is also important. I don't want an error from an invalid comment to show up as an error from an invalid integer literal.</p>
</blockquote>
<p>Why?</p>
<p>The usual recommendation is, to format your errors something like this:
<code>t.Errorf("Parse(%q) = %v, %v, want %v, %v", input, got, gotErr, want, wantErr)</code>, i.e. on a test-failure, you want to see what the input was and what the returned error strings where. If you can't tell from them, what the problem is, how are your users supposed to?</p>
<p>Furthermore, the sensible thing is to test the public API-contract you are providing; if specific error types are part of that public API, sure, you need to test them. If they aren't, then there are no tests needed. And, as above, if you truly believe that custom error types are needed to distinguish what's wrong, then your users need them too.</p>
<p>In short: a) Define your API. b) Test that API. This strategy is behavior-driven and a good litmus test whether you have a good API.</p></pre>smasher164: <pre><p>I agree with your advice on testing the public API, and to format errors in a readable way. But in this case, distinguishing between errors is intended for internally testing the parser. Although the user will see regular error messages for invalid input, I thought that relying on parsing strings to distinguish between two error messages would be bug-prone. This may or may not be the case. Consider this example:</p>
<pre><code>// Check the prefix of the error
if !strings.HasPrefix(err.Error(), "integer") {
t.Errorf("Wanted integer error, got %v", err)
}
</code></pre>
<p>While this may work for a specific case, both the breadth of different possible error values and the fact that the message is bound to a specific format make me uncomfortable. A slightly better option would be to return an error code, but then the public API would be forced to reflect that. If instead we create named types that embed an error, we can type assert to distinguish the error, albeit at the cost of extra type information.</p></pre>TheMerovius: <pre><blockquote>
<p>But in this case, distinguishing between errors is intended for internally testing the parser.</p>
</blockquote>
<p>Well, I understand the intention, I just don't agree that it's a good idea :)</p>
<blockquote>
<p>Although the user will see regular error messages for invalid input, I thought that relying on parsing strings to distinguish between two error messages would be bug-prone.</p>
</blockquote>
<p>You haven't answered the question why you need to distinguish them in the first place. What is the advantage of doing that in a test? I don't see any. Your testcases should be small enough, that the actual issue is obvious and the error message should describe the error well enough <em>anyway</em>, that it will give you a clear idea of what went wrong. I see zero advantage in distinguishing anything here.</p>
<p>And <em>if</em> there is any advantage, you very likely just want to export the errors in the first place, because apparently, for some reason, the correct usage of your API depends on distinguishing between different errors.</p>
<p>Maybe showing some code and some testcases would be a good idea to illustrate what advantage you see in distinguishing errors here? That would enable us to tell you whether you need to improve your error messages or whether there's genuinely a point in exporting the errors. But in any case, I do strongly believe, that adding a private API for your tests is the wrong thing to do here.</p></pre>jerf: <pre><p>I think the solution you are reaching for is that you need to implement an object that implements Error, but has all the data you need not in a string, so the correct implementation of <code>integerError</code> is:</p>
<pre><code>type integerError int
func (ie integerError) Error() string {
return fmt.Sprintf("Wanted integer error, got %v", ie)
}
</code></pre>
<p>That makes an <code>integerError{443}</code> an error.</p>
<p>Because errors are just interface values, if you use an internal type for them they are indistinguishable to the caller from any other errors <a href="https://golang.org/src/errors/errors.go?s=293:320#L1" rel="nofollow">generated by errors.New()</a>, which also is just a internal type. They won't be able to cast the error into the internal type, so they can't be tempted to do anything you didn't want. However, if you put your testing code into the same package it will be able to verify the error exactly. (In fact you can use something like <code>reflect.DeepEqual(err, integerError{443})</code> to do so, you don't even need a complicated chain of casting or anything.)</p>
<p>(Contrary to some people's advice, I <em>always</em> have my testing code in the same package, for this reason among many others. I use discipline to maintain separation and have decided I'll just wait until my testing code routinely goes stale due to worry about moving it to be strictly external based. Either I write testing code a lot differently than other people in a way that generally prevents its from going stale, or that concern is something that people theorize might happen, but doesn't happen often enough in practice to deny oneself the other advantages of having private access into testing time. The former is a legit possibility; I observe I also tend to create lots of relatively tiny packages for things that a lot of people make monolithic and it's possible that's what saves me.)</p>
<p>That said, for a parser I would see a case for a publicly-available "ParserError", which would be something like</p>
<pre><code>type ParserError struct {
Line int
Character int
Error error
Expected string
}
</code></pre>
<p>or something like that, because if you do make that stuff available it ought to be programmatically available. For a parser I'd likely make the guarantee that a call to the parser will either succeed OR produce a ParserError, guaranteed.</p>
<p>The reason I have an <code>Error error</code> in there is that if the parser takes an io.Reader, which it generally should, you also need a mechanism for returning the underlying error if one arises. </p></pre>Tikiatua: <pre><p>You could use interface definitions instead of specific types. See this article:
<a href="https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully" rel="nofollow">https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully</a></p></pre>smasher164: <pre><p>True, having a function that checks specific properties of an error can be useful. However, the error value would have to implement the interface, which would imply using a type that implements the error interface. In his example, he uses</p>
<pre><code>type temporary interface {
Temporary() bool
}
</code></pre>
<p>But the value that implements temporary would have to store sufficient information for the function to determine that it is temporary (like a property in a struct). An error value by itself is just a string, so you would need a custom type for it to be useful.</p></pre>
