Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
18 changes: 10 additions & 8 deletions .claude/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -209,14 +209,16 @@ and is what `internal/scanner/README.md#loader` cites for its figures.
loaders. `GOWORK` matters most: inside a workspace a sibling module resolves
to its working copy, and missing it synthesizes the sibling empty rather than
failing. See `internal/scanner/README.md#loader`.
- `SkipCompiledDependencies` — opt out (default false) of taking dependency
types from the compiler's export data under the go/packages loader. Unset is
the default since v0.36.4 and costs no meaning: a dependency's annotations are
read back after the load, and a declaration the spec needs is fetched at the
lookup that wants it. Set it for cost alone — `go list -export` compiles the
closure, so a cold cache is an order of magnitude slower and writes ~229 MB.
Code that fails to build needs no opt-out: that load is retried from source,
which is what keeps go-swagger#2874 fixed. See
- `CompiledDependencies` — opt in (default false) to taking dependency types
from the compiler's export data under the go/packages loader. Costs no
meaning: a dependency's annotations are read back after the load, and a
declaration the spec needs is fetched at the lookup that wants it. Set it for
cost alone, and only where the build cache is warm — `go list -export`
compiles the closure, so a cold cache is an order of magnitude slower and
writes ~229 MB, which is why the default protects the cold case. Code that
fails to build is safe either way: that load is retried from source, which is
what keeps go-swagger#2874 fixed. Asking for it under a loader that cannot
honour it raises a `scan.compiled-dependencies` Warning. See
`internal/scanner/README.md#compiled-dependencies`.
- Toolchain-free loading (experimental; see `internal/scanner/README.md#loader`):
- `ToolchainFreeLoader` — scan through `internal/packages` instead of
Expand Down
10 changes: 10 additions & 0 deletions .github/workflows/go-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ jobs:
test:
uses: go-openapi/ci-workflows/.github/workflows/go-test-monorepo.yml@81c95ab8bb6a3e0aeccc90a7c9e5fc5b398b1244 # v0.6.0
with:
# The suites emit the same document under any loader -- TestLoaderAgreement holds that over
# the whole corpus -- so which one runs is a question about what the run costs, and this
# harness is the shape compiled dependencies were made for: several hundred scans of small
# trees over one dependency closure, compiled once and read back by every scan after the
# first. Measured over the repository, that is 4m06s of CPU against 5m34s, and a runner with
# two cores spends most of the difference on the wall.
#
# A build tag rather than CODESCAN_TEST_LOADER because the shared workflow forwards flags to
# `go test` and not the environment. See internal/scantest for the knob itself.
extra-flags: -tags=testloader_compiled
# From v0.5.0 the shared workflow skips the matrix when nothing relevant changed, and
# what it watches is go sources. Ours are not the whole input: the integration tests
# compare against captured specs under testdata/integration/golden, and a corpus that
Expand Down
62 changes: 62 additions & 0 deletions .github/workflows/loader-conformance.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Runs the whole suite once per loader, so a divergence between them is caught as a divergence
# rather than as a puzzle.
#
# The loaders are meant to produce the same document by different means, and the agreement A/B
# asserts exactly that over the fixture corpus. It does not cover the SUITE: a test can depend on how
# its graph was loaded without saying so, which is how one measuring the compiled-dependency
# read-back economy came to fail under the toolchain-free loader, and how four sites wrapped twice in
# the run-wide loader setting passed every ordinary run.
#
# Deliberately none of what the shared go-test workflow does. No OS matrix, no -race, no coverage: on
# this question those add wall clock and variance and answer nothing, and windows in particular
# drowns the signal. One platform, one toolchain, plain `go test`. What it buys is a green tick per
# loader and three durations that can be read side by side.
name: loader conformance

on:
push:
branches:
- master

pull_request:

# On demand, for comparing the three after a change to how a scan loads anything.
workflow_dispatch:

permissions:
contents: read

jobs:
loaders:
name: ${{ matrix.loader }} loader
runs-on: ubuntu-latest
# Every loader reports, rather than the first failure hiding the other two: which ones diverge is
# the whole of what this job has to say.
strategy:
fail-fast: false
matrix:
loader:
- source
- compiled
- own
steps:
-
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
-
uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
with:
go-version: stable
check-latest: true
cache: true
cache-dependency-path: '**/go.sum'
-
name: Run the whole workspace under the ${{ matrix.loader }} loader
# The environment variable rather than the build tag the shared workflow has to use: a
# reusable workflow does not inherit the caller's environment, and this job is our own. It
# also means the variable is exercised somewhere, which the tags otherwise leave untested.
#
# -count=1 because a cached result reports no work and no duration, and the duration is half
# the point.
env:
CODESCAN_TEST_LOADER: ${{ matrix.loader }}
run: go test work -count=1 -timeout=30m ./...
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ internal/exportdata/exportdata.zip
/genspec-wasi
/loader-benchmark

# and from "go build ./..." run inside a command's own module, which drops the
# binary beside its main package rather than at the repo root
/cmd/genspec/genspec
/cmd/genspec-tui/genspec-tui
/cmd/genspec-wasi/genspec-wasi

# benchmark corpora: the archive is committed, the tree it unpacks to is not
internal/benchmarks/testdata/corpus/

Expand Down
4 changes: 2 additions & 2 deletions cmd/genspec-tui/internal/ux/diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,11 @@ func TestDoScanCollectsDiagnostics(t *testing.T) {
"types.go": badMaximumFixture,
})

res := scan.Do(&codescan.Options{
res := scan.Do(testutils.ApplyLoader(&codescan.Options{
WorkDir: dir,
Packages: []string{"."},
ScanModels: true,
}, &scan.Profiling{})
}), &scan.Profiling{})

if res.Err != nil {
t.Fatalf("scan should not hard-fail on soft diagnostics: %v", res.Err)
Expand Down
4 changes: 2 additions & 2 deletions cmd/genspec-tui/internal/ux/escape_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,12 @@ func TestSpecRender_EscapesControlCharacters(t *testing.T) {
"}\n"
require.NoError(t, os.WriteFile(filepath.Join(dir, "api.go"), []byte(src), 0o600))

res := scan.Do(&codescan.Options{
res := scan.Do(testutils.ApplyLoader(&codescan.Options{
WorkDir: dir,
Packages: []string{"./..."},
GOWORK: "off",
ScanModels: true,
}, nil)
}), nil)
require.NoError(t, res.Err)
require.Contains(t, res.JSON, "PWNED", "the crafted text never reached the document, so nothing is being tested")

Expand Down
3 changes: 2 additions & 1 deletion cmd/genspec-tui/internal/ux/fixtures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/go-openapi/codescan"
"github.com/go-openapi/codescan/cmd/genspec-tui/internal/index"
"github.com/go-openapi/codescan/cmd/genspec-tui/internal/ux/testutils"
"github.com/go-openapi/codescan/internal/parsers/grammar"
"github.com/go-openapi/codescan/internal/scanner"
)
Expand All @@ -31,7 +32,7 @@ func testModel(t *testing.T, opts ...modelOpt) *Model {
func testModelIn(t *testing.T, dir string, opts ...modelOpt) *Model {
t.Helper()

m := New(Startup{Options: &codescan.Options{WorkDir: dir, Packages: []string{"./..."}}})
m := New(Startup{Options: testutils.ApplyLoader(&codescan.Options{WorkDir: dir, Packages: []string{"./..."}})})
t.Cleanup(m.Close)

m.width, m.height = 80, 24
Expand Down
4 changes: 1 addition & 3 deletions cmd/genspec-tui/internal/ux/humanize/humanize.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ func SignedBytes(n int64) string {

// Negating MinInt64 overflows. Adding one before the negation keeps it in range, at the cost of a single byte at a
// magnitude no heap difference can reach.
//
//nolint:gosec // -(n+1) is non-negative for every n < 0, which is the only branch this is reached on
return "-" + Bytes(uint64(-(n+1))+1)
return "-" + Bytes(uint64(-(n+1))+1) //nolint:gosec // -(n+1) is non-negative for every n < 0, which is the only branch this is reached on
}

// SignedCount renders a count difference in short form: "+410 k", "-1.2 M", "+37".
Expand Down
5 changes: 3 additions & 2 deletions cmd/genspec-tui/internal/ux/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

"github.com/go-openapi/codescan"
"github.com/go-openapi/codescan/cmd/genspec-tui/internal/ux/scan"
"github.com/go-openapi/codescan/cmd/genspec-tui/internal/ux/testutils"
"github.com/go-openapi/codescan/internal/parsers/grammar"
"github.com/go-openapi/codescan/internal/scanner"
"github.com/go-openapi/testify/v2/assert"
Expand Down Expand Up @@ -762,11 +763,11 @@ func scanPetstore(t *testing.T) *Model {
t.Helper()

petstoreOnce.Do(func() {
petstoreRes = scan.Do(&codescan.Options{
petstoreRes = scan.Do(testutils.ApplyLoader(&codescan.Options{
WorkDir: fixturesDir(t),
Packages: []string{"./goparsing/petstore/..."},
ScanModels: true,
}, &scan.Profiling{})
}), &scan.Profiling{})
})
res := petstoreRes
require.NoError(t, res.Err, "the petstore fixture must scan cleanly")
Expand Down
4 changes: 2 additions & 2 deletions cmd/genspec-tui/internal/ux/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ func New(cfg *codescan.Options) Overlay {
&cfg.StubStdlib, &dep{&cfg.ToolchainFreeLoader, true, "ToolchainFreeLoader"},
},
{
groupLoading, "SkipCompiledDependencies", "read dep source, not the build cache",
&cfg.SkipCompiledDependencies, &dep{&cfg.ToolchainFreeLoader, false, "ToolchainFreeLoader"},
groupLoading, "CompiledDependencies", "use the build cache, not dep source",
&cfg.CompiledDependencies, &dep{&cfg.ToolchainFreeLoader, false, "ToolchainFreeLoader"},
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/genspec-tui/internal/ux/options/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var deliberatelyOmitted = map[string]string{ //nolint:gochecknoglobals // table
func newOverlay(t *testing.T, height int) (*Overlay, *codescan.Options) {
t.Helper()

cfg := &codescan.Options{WorkDir: t.TempDir(), Packages: []string{"./..."}}
cfg := testutils.ApplyLoader(&codescan.Options{WorkDir: t.TempDir(), Packages: []string{"./..."}})
o := New(cfg)
o.SetSize(100, height)

Expand Down
4 changes: 2 additions & 2 deletions cmd/genspec-tui/internal/ux/runstats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ func TestRunStats_TakesTheCostFromTheScan(t *testing.T) {
func TestRunStats_ProfiledRunNamesWhoSpentIt(t *testing.T) {
dir := t.TempDir()

res := scan.Do(&codescan.Options{
res := scan.Do(testutils.ApplyLoader(&codescan.Options{
WorkDir: fixturesDir(t),
Packages: []string{"./goparsing/classification/..."},
}, &scan.Profiling{Enabled: true, Dir: dir})
}), &scan.Profiling{Enabled: true, Dir: dir})
require.NoError(t, res.Err)

report := res.Profile
Expand Down
5 changes: 3 additions & 2 deletions cmd/genspec-tui/internal/ux/scan/scan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package scan
import (
"testing"

"github.com/go-openapi/codescan/cmd/genspec-tui/internal/ux/testutils"
"github.com/go-openapi/testify/v2/assert"
"github.com/go-openapi/testify/v2/require"

Expand All @@ -19,7 +20,7 @@ import (
func TestDoLeavesTheCallersOptionsAlone(t *testing.T) {
t.Parallel()

cfg := &codescan.Options{WorkDir: t.TempDir(), Packages: []string{"./..."}}
cfg := testutils.ApplyLoader(&codescan.Options{WorkDir: t.TempDir(), Packages: []string{"./..."}})

_ = Do(cfg, nil)

Expand All @@ -32,7 +33,7 @@ func TestDoLeavesTheCallersOptionsAlone(t *testing.T) {
func TestRunSnapshotsTheOptionsBeforeItRuns(t *testing.T) {
t.Parallel()

cfg := &codescan.Options{WorkDir: "/nonexistent-at-launch", Packages: []string{"./..."}}
cfg := testutils.ApplyLoader(&codescan.Options{WorkDir: "/nonexistent-at-launch", Packages: []string{"./..."}})

cmd := Run(cfg, nil)

Expand Down
4 changes: 2 additions & 2 deletions cmd/genspec-tui/internal/ux/source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -500,10 +500,10 @@ func classificationScan(t *testing.T) scan.ResultMsg {
t.Helper()

classificationOnce.Do(func() {
classificationRes = scan.Do(&codescan.Options{
classificationRes = scan.Do(testutils.ApplyLoader(&codescan.Options{
WorkDir: fixturesDir(t),
Packages: []string{"./goparsing/classification/..."},
}, &scan.Profiling{})
}), &scan.Profiling{})
})
require.NoError(t, classificationRes.Err)
require.NotEmpty(t, classificationRes.Diags, "the corpus must produce diagnostics")
Expand Down
11 changes: 6 additions & 5 deletions cmd/genspec-tui/internal/ux/startup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ import (

"github.com/go-openapi/codescan"
"github.com/go-openapi/codescan/cmd/genspec-tui/internal/ux/scan"
"github.com/go-openapi/codescan/cmd/genspec-tui/internal/ux/testutils"
)

// A session that behaves unlike the command line says it should is a session whose configuration file nobody knows
// about. It is said once, on the status line, and expires like every other notice.
func TestStartup_AnnouncesTheConfigurationFile(t *testing.T) {
m := New(Startup{
Options: &codescan.Options{WorkDir: t.TempDir(), Packages: []string{"./..."}},
Options: testutils.ApplyLoader(&codescan.Options{WorkDir: t.TempDir(), Packages: []string{"./..."}}),
ConfigPath: "/proj/.codescan.yaml",
ConfigSet: []string{"workdir", "scan-models"},
})
Expand All @@ -33,7 +34,7 @@ func TestStartup_AnnouncesTheConfigurationFile(t *testing.T) {

func TestStartup_SaysWhenTheFileDecidedNothing(t *testing.T) {
m := New(Startup{
Options: &codescan.Options{WorkDir: t.TempDir()},
Options: testutils.ApplyLoader(&codescan.Options{WorkDir: t.TempDir()}),
ConfigPath: "/proj/.codescan.yaml",
})
t.Cleanup(m.Close)
Expand All @@ -47,7 +48,7 @@ func TestStartup_SaysWhenTheFileDecidedNothing(t *testing.T) {
// One setting is one setting.
func TestStartup_CountsOneSettingSingly(t *testing.T) {
m := New(Startup{
Options: &codescan.Options{WorkDir: t.TempDir()},
Options: testutils.ApplyLoader(&codescan.Options{WorkDir: t.TempDir()}),
ConfigPath: "/proj/.codescan.yaml",
ConfigSet: []string{"workdir"},
})
Expand All @@ -61,7 +62,7 @@ func TestStartup_CountsOneSettingSingly(t *testing.T) {

// The ordinary case: no file, nothing said.
func TestStartup_SaysNothingWithoutAFile(t *testing.T) {
m := New(Startup{Options: &codescan.Options{WorkDir: t.TempDir()}})
m := New(Startup{Options: testutils.ApplyLoader(&codescan.Options{WorkDir: t.TempDir()})})
t.Cleanup(m.Close)

assert.Nil(t, m.announceConfig())
Expand All @@ -72,7 +73,7 @@ func TestStartup_SaysNothingWithoutAFile(t *testing.T) {
func TestStartup_CarriesProfilingToTheScans(t *testing.T) {
dir := t.TempDir()
m := New(Startup{
Options: &codescan.Options{WorkDir: dir},
Options: testutils.ApplyLoader(&codescan.Options{WorkDir: dir}),
Profiling: &scan.Profiling{Enabled: true, Dir: dir},
})
t.Cleanup(m.Close)
Expand Down
32 changes: 32 additions & 0 deletions cmd/genspec-tui/internal/ux/testutils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import (

tea "github.com/charmbracelet/bubbletea"
"github.com/go-openapi/testify/v2/require"

"github.com/go-openapi/codescan"
"github.com/go-openapi/codescan/internal/testloader"
)

// StripANSI removes the SGR escape sequences lipgloss emits.
Expand Down Expand Up @@ -41,3 +44,32 @@ func WriteTempGo(t *testing.T, content string) string {

return path
}

// ApplyLoader writes the loader this run was asked for onto opts, and returns it for chaining.
//
// The TUI scans through the same library as everything else, so its tests answer to the same
// run-wide setting; see internal/testloader for what selects it and why.
//
// A test that pins a loader must not route through here - there is no telling a field left at its
// zero value from one set on purpose - which is why this refuses one rather than writing over it.
func ApplyLoader(opts *codescan.Options) *codescan.Options {
if opts == nil || opts.FS != nil {
return opts
}
if opts.CompiledDependencies || opts.ToolchainFreeLoader {
panic("testutils: these options already pin a loader, so they must not be routed through " +
"ApplyLoader; drop the call and leave the pin, or drop the pin and keep the call")
}

switch selected := testloader.Selected(); selected {
case testloader.LoaderCompiled:
opts.CompiledDependencies = true
case testloader.LoaderOwn:
opts.ToolchainFreeLoader = true
case testloader.LoaderSource:
default:
panic("testutils: no rule for loader " + string(selected))
}

return opts
}
3 changes: 2 additions & 1 deletion cmd/genspec-tui/internal/ux/watcher/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ func (w *Watcher) Events() <-chan struct{} { return w.events }
func (w *Watcher) addRecursive(root string) {
_ = filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error {
if err != nil || !d.IsDir() {
return nil //nolint:nilerr // skip unreadable entries, keep walking
//nolint:nilerr // skip unreadable entries, keep walking
return nil
}
if name := d.Name(); path != root && (strings.HasPrefix(name, ".") || name == "vendor" || name == "node_modules") {
return filepath.SkipDir
Expand Down
7 changes: 6 additions & 1 deletion cmd/genspec-tui/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/go-openapi/codescan/cmd/internal/cliconf"
"github.com/go-openapi/codescan/cmd/internal/cliopts"
"github.com/go-openapi/codescan/internal/testloader"
"github.com/go-openapi/testify/v2/assert"
"github.com/go-openapi/testify/v2/require"
)
Expand Down Expand Up @@ -126,7 +127,11 @@ func exec(t *testing.T, argv ...string) (stdout, stderr string, err error) {
t.Helper()

var out, errs bytes.Buffer
err = run(argv, &out, &errs)
const maxExtraArgs = 4
argsWithLoader := make([]string, 0, len(argv)+maxExtraArgs)
argsWithLoader = append(argsWithLoader, testloader.Flags()...)
argsWithLoader = append(argsWithLoader, argv...)
err = run(argsWithLoader, &out, &errs)

return out.String(), errs.String(), err
}
Expand Down
Loading
Loading