From 391fc97fca932a50557d8e0f07ea2dfa8135900a Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Tue, 25 Aug 2026 15:54:53 +0200 Subject: ci: fail the build when documentation falls behind Adding a flag and forgetting the man page was the kind of thing only a reader would catch, and the README had already drifted: it documented seven of fifteen flags, claimed three cross-compilation targets where the Makefile builds six, and its example output predated the humidity column. Flag definitions move into defineFlags(), so the test enumerates the same set run() does rather than a hand-copied list that could drift in its own right. usage() gains a writer so its output can be captured. Tests then assert that every flag reaches -h, the README and the man page; that every key written into the generated config is documented; and that every column name is explained. The man page check normalises roff's \- hyphen escape first -- without that, every multi-word flag looks undocumented when it is not. --- CHANGELOG.md | 7 ++- cmd/prognosis/docs_test.go | 122 +++++++++++++++++++++++++++++++++++++++++++++ cmd/prognosis/main.go | 106 ++++++++++++++++++++++----------------- 3 files changed, 188 insertions(+), 47 deletions(-) create mode 100644 cmd/prognosis/docs_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index c797f5e..6806130 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,8 +15,11 @@ Terse, newest first. Versions are git tags. - `-version`, and the version is stamped into the binary at build time. - `-weather` (forecast only), `-no-warnings`, `-ascii` (GSM-7 safe output for SMS). -- `make lint` checks the man page renders without groff warnings; `make ci` - now includes it. `make release`, `make install-hooks`, and cross-compilation +- `make lint` checks the man page renders without groff warnings, and tests + assert that every flag appears in `-h`, the README and the man page, that + every config key the generated file contains is documented, and that every + column name is explained in both. Documentation drift now fails `make ci` + rather than reaching a release. `make release`, `make install-hooks`, and cross-compilation for six platforms. ### Changed diff --git a/cmd/prognosis/docs_test.go b/cmd/prognosis/docs_test.go new file mode 100644 index 0000000..6aa97c0 --- /dev/null +++ b/cmd/prognosis/docs_test.go @@ -0,0 +1,122 @@ +package main + +import ( + "bytes" + "flag" + "io" + "os" + "regexp" + "strings" + "testing" + + "github.com/lukaszkasprzak/prognosis/internal/config" +) + +func readRepoFile(t *testing.T, rel string) string { + t.Helper() + // Tests run in the package directory; the docs live at the repo root. + b, err := os.ReadFile("../../" + rel) + if err != nil { + t.Fatalf("cannot read %s: %v", rel, err) + } + // roff escapes a literal hyphen as \- , so "-no-warnings" is written + // "\-no\-warnings". Undo that before searching, or every multi-word flag + // looks undocumented when it is not. + return strings.ReplaceAll(string(b), `\-`, "-") +} + +// Every flag must appear in -h, in the README and in the man page. +// +// The flag set comes from defineFlags, the same function run() uses, so this +// cannot be satisfied by a stale hand-written list: adding a flag and +// forgetting to document it fails the build. +func TestEveryFlagIsDocumented(t *testing.T) { + fs := flag.NewFlagSet("prognosis", flag.ContinueOnError) + fs.SetOutput(io.Discard) + defineFlags(fs) + + var help bytes.Buffer + usageTo(&help) + + docs := map[string]string{ + "-h": help.String(), + "README.md": readRepoFile(t, "README.md"), + "man/prognosis.1": readRepoFile(t, "man/prognosis.1"), + } + + fs.VisitAll(func(f *flag.Flag) { + for where, text := range docs { + if !strings.Contains(text, "-"+f.Name) { + t.Errorf("flag -%s is not documented in %s", f.Name, where) + } + } + }) +} + +// The reverse: -h must not advertise a flag that does not exist, which would +// send someone chasing a typo. +func TestHelpAdvertisesNoPhantomFlags(t *testing.T) { + fs := flag.NewFlagSet("prognosis", flag.ContinueOnError) + fs.SetOutput(io.Discard) + defineFlags(fs) + + real := map[string]bool{} + fs.VisitAll(func(f *flag.Flag) { real[f.Name] = true }) + + var help bytes.Buffer + usageTo(&help) + for _, m := range regexp.MustCompile(`(?m)^ -([a-z-]+)`).FindAllStringSubmatch(help.String(), -1) { + if !real[m[1]] { + t.Errorf("-h lists -%s, which is not a real flag", m[1]) + } + } +} + +// Every key the generated config file contains must be documented in the man +// page. The generated file is the authoritative list of user-facing settings, +// so this catches a new key that never reached the documentation. +func TestEveryConfigKeyIsDocumented(t *testing.T) { + path := t.TempDir() + "/config" + if err := config.WriteDefault(path, config.Default()); err != nil { + t.Fatal(err) + } + generated, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + man := readRepoFile(t, "man/prognosis.1") + + seen := map[string]bool{} + for _, line := range strings.Split(string(generated), "\n") { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + key, _, ok := strings.Cut(line, "=") + if !ok || seen[key] { + continue + } + seen[key] = true + if !strings.Contains(man, key) { + t.Errorf("config key %q is written into the generated config but not documented in the man page", key) + } + } + if len(seen) < 8 { + t.Fatalf("only found %d config keys; the parser above is probably wrong", len(seen)) + } +} + +// The columns a user can name must all be documented, or the error message +// listing them points at something the man page never explains. +func TestEveryColumnIsDocumented(t *testing.T) { + man := readRepoFile(t, "man/prognosis.1") + readme := readRepoFile(t, "README.md") + for _, col := range config.ValidColumns() { + if !strings.Contains(man, col) { + t.Errorf("column %q is not documented in the man page", col) + } + if !strings.Contains(readme, col) { + t.Errorf("column %q is not documented in the README", col) + } + } +} diff --git a/cmd/prognosis/main.go b/cmd/prognosis/main.go index 3075563..99d9918 100644 --- a/cmd/prognosis/main.go +++ b/cmd/prognosis/main.go @@ -6,6 +6,7 @@ import ( "errors" "flag" "fmt" + "io" "os" "strconv" "strings" @@ -18,6 +19,35 @@ import ( "github.com/lukaszkasprzak/prognosis/internal/render" ) +// options is every command line flag. Defined in one place so that run() and +// the test asserting each one is documented cannot disagree about the set. +type options struct { + location, columns, icons, lang, pollen *string + hours, days, pick *int + noGraph, weather, noWarn, asciiOut, noColor *bool + showCfg, showVer *bool +} + +func defineFlags(fs *flag.FlagSet) *options { + return &options{ + location: fs.String("l", "", "place to query (default: location= in the config file)"), + hours: fs.Int("n", 0, "hours ahead to show"), + days: fs.Int("d", 0, "days ahead to show, 24h each"), + columns: fs.String("columns", "", "comma-separated columns to display"), + icons: fs.String("icons", "", "icon set: nerd, emoji or none"), + lang: fs.String("lang", "", "display language: en or pl"), + pollen: fs.String("pollen", "", "pollen species to show: a list, or all / none"), + noGraph: fs.Bool("no-graph", false, "table only, no chart"), + weather: fs.Bool("weather", false, "forecast only: no sun times, summary, pollen or chart"), + noWarn: fs.Bool("no-warnings", false, "omit IMGW warnings"), + asciiOut: fs.Bool("ascii", false, "ASCII only, so an SMS stays in GSM-7"), + noColor: fs.Bool("no-color", false, "plain output"), + pick: fs.Int("pick", 0, "choose the Nth place when the name is ambiguous"), + showCfg: fs.Bool("config", false, "print the config file path and exit"), + showVer: fs.Bool("version", false, "print the version and exit"), + } +} + // version is stamped at build time: -ldflags "-X main.version=$(git describe)". // "dev" means someone built it straight from a working tree. var version = "dev" @@ -28,23 +58,7 @@ func run() int { // Android has no /etc/resolv.conf; without this every lookup fails. configureResolver() - var ( - location = flag.String("l", "", "place to query (default: location= in the config file)") - hours = flag.Int("n", 0, "hours ahead to show") - days = flag.Int("d", 0, "days ahead to show, 24h each") - columns = flag.String("columns", "", "comma-separated columns to display") - icons = flag.String("icons", "", "icon set: nerd, emoji or none") - lang = flag.String("lang", "", "display language: en or pl") - pollen = flag.String("pollen", "", "pollen species to show: a list, or all / none") - noGraph = flag.Bool("no-graph", false, "table only, no chart") - weather = flag.Bool("weather", false, "forecast only: no sun times, summary, pollen or chart") - noWarn = flag.Bool("no-warnings", false, "omit IMGW warnings") - asciiOut = flag.Bool("ascii", false, "ASCII only, so an SMS stays in GSM-7") - noColor = flag.Bool("no-color", false, "plain output") - pick = flag.Int("pick", 0, "choose the Nth place when the name is ambiguous") - showCfg = flag.Bool("config", false, "print the config file path and exit") - showVer = flag.Bool("version", false, "print the version and exit") - ) + opt := defineFlags(flag.CommandLine) flag.Usage = usage // Go's flag package stops at the first non-flag argument, so // "prognosis 52.52,13.40 -n 3" would swallow the flags into the place name. @@ -62,13 +76,13 @@ func run() int { rest = flag.Args()[1:] } - if *showVer { + if *opt.showVer { fmt.Printf("prognosis %s\n", version) return 0 } cfgPath := config.Path() - if *showCfg { + if *opt.showCfg { fmt.Println(cfgPath) return 0 } @@ -86,66 +100,66 @@ func run() int { } // Flags override the file. The place is settled later, by placeFromArgs. - if *columns != "" { - cfg.Columns = splitList(*columns) + if *opt.columns != "" { + cfg.Columns = splitList(*opt.columns) } - if *icons != "" { - cfg.Icons = *icons + if *opt.icons != "" { + cfg.Icons = *opt.icons } - if *lang != "" { - cfg.DisplayLang = *lang + if *opt.lang != "" { + cfg.DisplayLang = *opt.lang } - if *pollen != "" { - switch *pollen { + if *opt.pollen != "" { + switch *opt.pollen { case "all": cfg.Pollen, cfg.PollenExplicit = append([]string(nil), config.AllSpecies...), false case "none": cfg.Pollen, cfg.PollenExplicit = nil, false default: - cfg.Pollen, cfg.PollenExplicit = splitList(*pollen), true + cfg.Pollen, cfg.PollenExplicit = splitList(*opt.pollen), true } } - if *noGraph { + if *opt.noGraph { cfg.Graph = false } - if *weather { + if *opt.weather { // Meant for piping to someone else: the forecast and nothing else. cfg.Minimal = true cfg.Graph = false } - if *noWarn { + if *opt.noWarn { cfg.Warnings = false } - if *asciiOut { + if *opt.asciiOut { cfg.ASCII = true } if cfg.ASCII { // Both glyph sets are non-ASCII by definition. cfg.Icons = "none" } - if *noColor { + if *opt.noColor { cfg.Color = "never" } switch { - case *days > 0 && *hours > 0: + case *opt.days > 0 && *opt.hours > 0: fmt.Fprintln(os.Stderr, "prognosis: -n and -d cannot be combined") return 2 - case *days > 0: - if *days > openmeteo.MaxForecastDays-1 { + case *opt.days > 0: + if *opt.days > openmeteo.MaxForecastDays-1 { fmt.Fprintf(os.Stderr, "prognosis: -d must be between 1 and %d\n", openmeteo.MaxForecastDays-1) return 2 } - cfg.Hours = *days * 24 - case *hours > 0: - cfg.Hours = *hours - case *days < 0 || *hours < 0: + cfg.Hours = *opt.days * 24 + case *opt.hours > 0: + cfg.Hours = *opt.hours + case *opt.days < 0 || *opt.hours < 0: fmt.Fprintln(os.Stderr, "prognosis: hours and days must be positive") return 2 } // -icons only chooses what the icon column draws. Without that column it // changes nothing, which looks like the flag being ignored. - if *icons != "" && !cfg.Has("icon") { + if *opt.icons != "" && !cfg.Has("icon") { fmt.Fprintf(os.Stderr, "note: -icons has no effect: %q is not in columns (add it: -columns %s)\n", "icon", strings.Join(append([]string{"hour", "icon"}, cfg.Columns[1:]...), ",")) @@ -156,7 +170,7 @@ func run() int { return 2 } - place, err := placeFromArgs(*location, positional) + place, err := placeFromArgs(*opt.location, positional) if err != nil { fmt.Fprintf(os.Stderr, "prognosis: %v\n", err) return 2 @@ -172,7 +186,7 @@ func run() int { } store := cache.New(cache.DefaultPath()) - geo, err := resolve(store, cfg.Location, *pick) + geo, err := resolve(store, cfg.Location, *opt.pick) if err != nil { var amb *ambiguousError if errors.As(err, &amb) { @@ -419,8 +433,10 @@ func splitList(v string) []string { return out } -func usage() { - fmt.Fprintf(os.Stderr, `prognosis - hour-by-hour forecast, with IMGW warnings for Poland +func usage() { usageTo(os.Stderr) } + +func usageTo(w io.Writer) { + fmt.Fprintf(w, `prognosis - hour-by-hour forecast, with IMGW warnings for Poland usage: prognosis [flags] [place] -- cgit v1.3