diff --git a/go/best-practices.md b/go/best-practices.md index 1c401f6..29c2ad4 100644 --- a/go/best-practices.md +++ b/go/best-practices.md @@ -165,7 +165,7 @@ methods: -### Test double packages and types +### Test double and helper packages There are several disciplines you can apply to [naming] packages and types that provide test helpers and especially [test doubles]. A test double could be a @@ -1241,11 +1241,47 @@ Another case in which panics can be useful, though uncommon, is as an internal implementation detail of a package which always has a matching recover in the callchain. Parsers and similar deeply nested, tightly coupled internal function groups can benefit from this design, where plumbing error returns adds -complexity without value. The key attribute of this design is that these panics -are never allowed to escape across package boundaries and do not form part of -the package's API. This is typically accomplished with a top-level deferred -recover that translates a propagating panic into a returned error at the public -API surfaces. +complexity without value. + +The key attribute of this design is that these **panics are never allowed to +escape across package boundaries** and do not form part of the package's API. +This is typically accomplished with a top-level deferred function that uses +`recover` to translate a propagated panic into a returned error at the public +API boundary. It requires the code that panics and recovers to distinguish +between panics that the code raises itself and those that it doesn't: + +```go +// Good: +type syntaxError struct { + msg string +} + +func parseInt(in string) int { + n, err := strconv.Atoi(in) + if err != nil { + panic(&syntaxError{"not a valid integer"}) + } +} + +func Parse(in string) (_ *Node, err error) { + defer func() { + if p := recover(); p != nil { + sErr, ok := p.(*syntaxError) + if !ok { + panic(p) // Propagate the panic since it is outside our code's domain. + } + err = fmt.Errorf("syntax error: %v", sErr.msg) + } + }() + ... // Parse input calling parseInt internally to parse integers +} +``` + +> **Warning:** Code employing this pattern must take care to manage any +> resources associated with the code run in such defer-managed sections (e.g., +> close, free, or unlock). +> +> See: [Go Tip #81: Avoiding Resource Leaks in API Design] Panic is also used when the compiler cannot identify unreachable code, for example when using a function like `log.Fatal` that will not return: @@ -1270,6 +1306,16 @@ If you must die in a package initialization function (an `init` or a ["must" function](decisions#must-functions)), a panic is acceptable in place of the fatal logging call. +See also: + +* [Handling panics](https://go.dev/ref/spec#Handling_panics) and + [Run-time Panics](https://go.dev/ref/spec#Run_time_panics) in the language + specification +* [Defer, Panic, and Recover](https://go.dev/blog/defer-panic-and-recover) +* [On the uses and misuses of panics in Go](https://eli.thegreenplace.net/2018/on-the-uses-and-misuses-of-panics-in-go/) + +[Go Tip #81: Avoiding Resource Leaks in API Design]: https://google.github.io/styleguide/go/index.html#gotip + ## Documentation diff --git a/go/decisions.md b/go/decisions.md index 33e6ee5..38084c7 100644 --- a/go/decisions.md +++ b/go/decisions.md @@ -96,8 +96,15 @@ must be renamed at import time to a name that is suitable for use in Go code. An exception to this is that package names that are only imported by generated code may contain underscores. Specific examples include: -* Using the `_test` suffix for an external test package, for example an - integration test +* Using the `_test` suffix for unit tests that only exercise the exported API + of a package (package `testing` calls these + ["black box tests"](https://pkg.go.dev/testing)). For example, a package + `linkedlist` must define its black box unit tests in a package named + `linkedlist_test` (not `linked_list_test`) + +* Using underscores and the `_test` suffix for packages that specify + functional or integration tests. For example, a linked list service + integration test could be named `linked_list_service_test` * Using the `_test` suffix for [package-level documentation examples](https://go.dev/blog/examples) @@ -3401,11 +3408,15 @@ command line for users of test filtering. When you use `t.Run` to create a subtest, the first argument is used as a descriptive name for the test. To ensure that test results are legible to humans reading the logs, choose subtest names that will remain useful and readable after escaping. Think of subtest -names more like a function identifier than a prose description. The test runner -replaces spaces with underscores, and escapes non-printing characters. If your -test data benefits from a longer description, consider putting the description -in a separate field (perhaps to be printed using `t.Log` or alongside failure -messages). +names more like a function identifier than a prose description. + +The test runner replaces spaces with underscores, and escapes non-printing +characters. To ensure accurate correlation between test logs and source code, it +is recommended to avoid using these characters in subtest names. + +If your test data benefits from a longer description, consider putting the +description in a separate field (perhaps to be printed using `t.Log` or +alongside failure messages). Subtests may be run individually using flags to the [Go test runner] or Bazel [test filter], so choose descriptive names that are also easy to type.