From f3b2f9b0ef09ed20119f5b89a9652b14ccd94122 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sat, 25 Apr 2026 01:14:56 +0000 Subject: [PATCH] all: fix duplicate package docs and tighten TestPackageDocs TestPackageDocs walked into directories starting with "." (such as .claude worktrees) and only logged warnings on duplicate package docs across files in a directory. Skip dot-directories (which covers the old .git but also .claude), ignore files with "//go:build ignore" so command files don't falsely trip the duplicate check, and promote the duplicate-doc warning to a t.Errorf. While here, deduplicate the package docs that were previously only logged: drop the redundant comment from client/systray/startup-creator.go, move the comprehensive taildrop doc into feature/taildrop/doc.go, and remove a leftover doc fragment from feature/condlite/expvar/omit.go. The tstest/integration/vms allowlist is no longer needed since the //go:build ignore filter now handles its dns_tester.go and udp_tester.go files generically. Fixes #19526 Change-Id: Id794d96bd728826a1883a054e4a244f90fa05d3d Signed-off-by: Brad Fitzpatrick --- client/systray/startup-creator.go | 1 - feature/condlite/expvar/omit.go | 1 - feature/taildrop/doc.go | 7 ++++++- feature/taildrop/taildrop.go | 6 ------ pkgdoc_test.go | 27 ++++++++++++++++++--------- 5 files changed, 24 insertions(+), 18 deletions(-) diff --git a/client/systray/startup-creator.go b/client/systray/startup-creator.go index 369190012..02a018099 100644 --- a/client/systray/startup-creator.go +++ b/client/systray/startup-creator.go @@ -3,7 +3,6 @@ //go:build cgo || !darwin -// Package systray provides a minimal Tailscale systray application. package systray import ( diff --git a/feature/condlite/expvar/omit.go b/feature/condlite/expvar/omit.go index b5481695c..188de2af2 100644 --- a/feature/condlite/expvar/omit.go +++ b/feature/condlite/expvar/omit.go @@ -3,7 +3,6 @@ //go:build ts_omit_debug && ts_omit_clientmetrics && ts_omit_usermetrics -// excluding the package from builds. package expvar type Int int64 diff --git a/feature/taildrop/doc.go b/feature/taildrop/doc.go index c394ebe82..a3243b3c2 100644 --- a/feature/taildrop/doc.go +++ b/feature/taildrop/doc.go @@ -1,5 +1,10 @@ // Copyright (c) Tailscale Inc & contributors // SPDX-License-Identifier: BSD-3-Clause -// Package taildrop registers the taildrop (file sending) feature. +// Package taildrop contains the implementation of the Taildrop +// functionality including sending and retrieving files. +// This package does not validate permissions, the caller should +// be responsible for ensuring correct authorization. +// +// For related documentation see: http://go/taildrop-how-does-it-work package taildrop diff --git a/feature/taildrop/taildrop.go b/feature/taildrop/taildrop.go index 7042ca97a..9839b8330 100644 --- a/feature/taildrop/taildrop.go +++ b/feature/taildrop/taildrop.go @@ -1,12 +1,6 @@ // Copyright (c) Tailscale Inc & contributors // SPDX-License-Identifier: BSD-3-Clause -// Package taildrop contains the implementation of the Taildrop -// functionality including sending and retrieving files. -// This package does not validate permissions, the caller should -// be responsible for ensuring correct authorization. -// -// For related documentation see: http://go/taildrop-how-does-it-work package taildrop import ( diff --git a/pkgdoc_test.go b/pkgdoc_test.go index 60b2d4856..c5e50ee63 100644 --- a/pkgdoc_test.go +++ b/pkgdoc_test.go @@ -4,6 +4,7 @@ package tailscaleroot import ( + "go/ast" "go/parser" "go/token" "os" @@ -13,6 +14,17 @@ import ( "testing" ) +func hasIgnoreBuildTag(f *ast.File) bool { + for _, cg := range f.Comments { + for _, c := range cg.List { + if c.Text == "//go:build ignore" { + return true + } + } + } + return false +} + func TestPackageDocs(t *testing.T) { switch runtime.GOOS { case "darwin", "linux": @@ -26,8 +38,8 @@ func TestPackageDocs(t *testing.T) { if err != nil { return err } - if fi.Mode().IsDir() && path == ".git" { - return filepath.SkipDir // No documentation lives in .git + if fi.Mode().IsDir() && path != "." && strings.HasPrefix(filepath.Base(path), ".") { + return filepath.SkipDir // No documentation lives in dot directories (.git, .claude, etc) } if fi.Mode().IsRegular() && strings.HasSuffix(path, ".go") { if strings.HasSuffix(path, "_test.go") { @@ -48,6 +60,9 @@ func TestPackageDocs(t *testing.T) { if err != nil { t.Fatalf("failed to ParseFile %q: %v", fileName, err) } + if hasIgnoreBuildTag(f) { + continue + } dir := filepath.Dir(fileName) if _, ok := byDir[dir]; !ok { byDir[dir] = nil @@ -61,14 +76,8 @@ func TestPackageDocs(t *testing.T) { } } for dir, ff := range byDir { - switch dir { - case "tstest/integration/vms": - // This package has a couple go:build ignore commands and this test doesn't - // handle parsing those. Just allowlist that package for now (2024-07-10). - continue - } if len(ff) > 1 { - t.Logf("multiple files with package doc in %s: %q", dir, ff) + t.Errorf("multiple files with package doc in %s: %q", dir, ff) } if len(ff) == 0 { if strings.HasPrefix(dir, "gokrazy/") {