-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
testing: support unordered output in Examples. #10149
Comments
This is an interesting idea. So the testing framework would sort the lines of the output and expected output before comparing? Can you point toward a real-world use of this that might serve as a motivating example? |
I am uncomfortable about this. Map ordering is undefined, and it's an uphill battle to dispel the myth that newer versions of Go changed the map ordering semantics (they were never guaranteed in first place), so I am not keen on this feature, no matter how well meaning. |
@davecheney this feature seems to specifically call out that some operations will have unordered output, which is exactly the consideration we want developers to keep in mind when using maps. With that said, I'm not sold on the feature, either. That's why I asked for more info. |
Initially when I started working on this I was thinking about how I would document a package like "shuffle" which works like "sort" except that it shuffles items around randomly rather than sorting them [think python random.shuffle()]. Basically an example of this function with Output is completely impossible to show since by definition the output is randomized. Being able to show the expected output, with a warning that it specifically can be in any order is useful in my opinion. So the function to test shuffle.Shuffle() could look like this: func ExampleShuffle() {
x := []int{1, 2, 3, 4, 5}
shuffle.ShuffleInt(x)
for index, value := range x {
fmt.Printf("index[%d] = %d\n", index, value)
}
// Unordered output:
// 4
// 2
// 5
// 1
// 3
} Other use cases are like the one I am specifically working on now. A client I work on makes calls to an external service which returns results in an unspecified order. Ideally I don't want to have to sort the output myself here since every call might end up having the same boiler plate around it, therefor making the examples far more complicated to understand. I suspect that this is common in the Example realm specifically because of things like non-repeatable ordering in maps. All that being said, I actually love the assertion of lack of assured order. Its awesome because it makes clear the only promise that can be made. It does however make things like "fixed output" testing more complicated. I would love to see changes like this because it embraces the assurance of "unordered" in the example to make it clear what is expected. [If you want I can list off examples of existing library calls that would be difficult to document using the existing '// Output:' mode.. say os.(*File).Readdir(n int) since the results depend on the underlying file system calls, most the functionality in atomic since it necessitates go routines, and likely a few more I can't think of off the top of my head.] |
I think this is a pretty reasonable feature request. What does @robpike think? |
I like this idea as long as it can be kept simple; and @liquidgecka 's example implies a pretty simple implementation:
Thus, the example checker would have to verify that any runtime output is a permutation of the "unordered output" which is easy enough. The one question I have: Is this too restrictive for practical use? |
I want to propose that go test check that different invocations of the This is to prevent this being used when the result is deterministic |
@minux My worry with that would be that some conditions might make it hard to ensure two runs will have different ordering. For example, the Readdir() example above might return the same order based on file insertion, or based on file name hashes, etc. This depends on the underlying file system implementation (ext vs tmpfs and such) and therefor two runs with the same process and temp directory would return the same results even though the output is still not order assured. Making the test require the output to change might add complexity and introduce possible false failures that could be confusing to somebody debugging. |
This is correct. Map ordering is "unspecified", not "guarenteed to be On Fri, Mar 13, 2015 at 4:24 PM, Brady Catherman notifications@github.com
|
Then how to prevent the example author to always use "Unordered output:"
when "Output:" is fine?
We go to great length to make sure map range order is not the same across
runs, even at the expense of slight performance hit.
|
On Fri, Mar 13, 2015 at 4:33 PM, Minux Ma notifications@github.com wrote:
I don't believe this feature should be added. As you point out, it will be
I know, and I have said previously that I think this is a mistake. But that
|
I was going to make gri's suggestion before I saw his, a different prefix that says the output may be unsorted. "Output" means this is what you get. Seems reasonable and easy, but I do worry a bit about the first step onto a slippery slope. It's possible this is the only step we'll take, though. So cautiously favorable but not convinced. |
I think my final pitch for it would be this: One of golang's most awesome features is its ability to run Examples as tests. This proposal basically extends those capabilities to cases where the output might not have been predictable before without also adding significant complexity or new knowledge requirements to use it. It also allows an API to more clearly emphasize return results as unordered which can help sway confusion, much in the same way that the changes to map[] forced people to understand the unordered nature of it. While this would be minimal for implementation side, it could really help on the documentation, and testing side of the house. I love tested/documented code, and anything simple that makes writing tests and documentation in makes it easier to complain about those that don't actually take the time to do it! =) |
On 13 March 2015 at 16:36, Dave Cheney notifications@github.com wrote:
If the thing you're demonstrating does emit things in an arbitrary order, I'm having trouble understanding your objection. |
My objection mirrors Minux's. I worry that the temptation to use "Unsorted Put simply, I don't see the very rare case of documenting the output of a On Mon, Mar 16, 2015 at 1:41 PM, Andrew Gerrand notifications@github.com
|
This seems like an okay thing. |
Will have to wait for Go 1.6 |
Still seems okay to do. |
I would prefer "Unordered" to "Unsorted" as the term -- "unordered" is broader, and has fewer (mis)implications about the underlying matching algorithm. |
+1 on "Unordered". Typical test output is not "sorted". |
yea unordered was what I used in an example and is the proper term for this so +1 to that wording. |
I took a very, very terrible stab at this here: https://go-review.googlesource.com/19280 Its both incomplete and likely downright awful. Please skewer the author with anything that can be done to make this better. I also didn't get to godoc yet which would be necessary I suspect to making this "real" since otherwise it removes the advantage of the visibility in the documentation. |
CL https://golang.org/cl/19280 mentions this issue. |
I am not sure how to re-open this, but its not actually done yet as I need to implement the changes in godoc now that the testing package supports the format and all that. |
Reopened. |
I wasn't sure how to cross-link from tools to here, but the change for godoc is here: https://go-review.googlesource.com/#/c/20458/ Thanks so much @adg for walking me through this process. =) |
Is there still follow-up work remaining? The last thing I can find is a comment from @adg on https://go-review.googlesource.com/#/c/20492/:
|
CL https://golang.org/cl/22032 mentions this issue. |
A pared down verison of https://golang.org/cl/20458 that is compatible with Go 1.5. Fixes golang/go#10149 Change-Id: I550b097dc1cd65d0711bdea943f2b0678cfbc5fd Reviewed-on: https://go-review.googlesource.com/22032 Reviewed-by: Caleb Spare <cespare@gmail.com> Reviewed-by: Andrew Gerrand <adg@golang.org> Run-TryBot: Andrew Gerrand <adg@golang.org>
Ideally tests should support unordered output as well as regular output in testing so that every example that uses maps doesn't need to make an output array and sort it (therefor adding complexity to unrelated example code.) This is very useful when dealing with maps which by design return unordered iteration patterns.
An example of the syntax I am proposing (Or something like that):
In this case the output on godoc would show the above, while the test would pass regardless of which order the lines were outputted.
The text was updated successfully, but these errors were encountered: