Skip to content
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.

Sign up for GitHub

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

feat: configurable batch size #458

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
+518 −171
Draft

feat: configurable batch size #458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5f4c98b
feat: improve formatter/config change detection
brianmcgee Oct 17, 2024
b9c0568
chore: remove unused code
brianmcgee Oct 17, 2024
fd59969
feat: configurable batch size
brianmcgee Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions cmd/format/format.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ import (
bolt "go.etcd.io/bbolt"
)

const (
BatchSize = 1024
)

var ErrFailOnChange = errors.New("unexpected changes detected, --fail-on-change is enabled")

func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string) error {
Expand Down Expand Up @@ -156,7 +152,7 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string)
}

// create a composite formatter which will handle applying the correct formatters to each file we traverse
formatter, err := format.NewCompositeFormatter(cfg, statz, BatchSize)
formatter, err := format.NewCompositeFormatter(cfg, statz)
if err != nil {
return fmt.Errorf("failed to create composite formatter: %w", err)
}
Expand All @@ -175,7 +171,7 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string)
}

// start traversing
files := make([]*walk.File, BatchSize)
files := make([]*walk.File, cfg.BatchSize)

for {
// read the next batch
Expand Down
138 changes: 132 additions & 6 deletions cmd/root_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestOnUnmatched(t *testing.T) {

checkOutput := func(level string, output []byte) {
for _, p := range paths {
as.Contains(string(output), fmt.Sprintf("%s format: no formatter for path: %s", level, p))
as.Contains(string(output), fmt.Sprintf("%s composite-formatter: no formatter for path: %s", level, p))
}
}

Expand Down Expand Up @@ -112,6 +112,41 @@ func TestCpuProfile(t *testing.T) {
as.NoError(err)
}

func TestBatchSize(t *testing.T) {
as := require.New(t)
tempDir := test.TempExamples(t)

// capture current cwd, so we can replace it after the test is finished
cwd, err := os.Getwd()
as.NoError(err)

t.Cleanup(func() {
// return to the previous working directory
as.NoError(os.Chdir(cwd))
})

_, _, err = treefmt(t, "-C", tempDir, "--allow-missing-formatter", "--batch-size", "10241")
as.ErrorIs(err, config.ErrInvalidBatchSize)

_, _, err = treefmt(t, "-C", tempDir, "--allow-missing-formatter", "--batch-size", "-10241")
as.ErrorContains(err, "invalid argument")

_, _, err = treefmt(t, "-C", tempDir, "--allow-missing-formatter")
as.NoError(err)

out, _, err := treefmt(t, "-C", tempDir, "--allow-missing-formatter", "--batch-size", "1", "-v")
as.NoError(err)
as.Contains(string(out), fmt.Sprintf("INFO config: batch size = %d", 1))

out, _, err = treefmt(t, "-C", tempDir, "--allow-missing-formatter", "--batch-size", "4", "-v")
as.NoError(err)
as.Contains(string(out), fmt.Sprintf("INFO config: batch size = %d", 4))

out, _, err = treefmt(t, "-C", tempDir, "--allow-missing-formatter", "--batch-size", "128", "-v")
as.NoError(err)
as.Contains(string(out), fmt.Sprintf("INFO config: batch size = %d", 128))
}

func TestAllowMissingFormatter(t *testing.T) {
as := require.New(t)

Expand Down Expand Up @@ -605,15 +640,15 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
configPath := tempDir + "/touch.toml"

// symlink some formatters into temp dir, so we can mess with their mod times
binPath := tempDir + "/bin"
binPath := filepath.Join(tempDir, "bin")
as.NoError(os.Mkdir(binPath, 0o755))

binaries := []string{"black", "elm-format", "gofmt"}

for _, name := range binaries {
src, err := exec.LookPath(name)
as.NoError(err)
as.NoError(os.Symlink(src, binPath+"/"+name))
as.NoError(os.Symlink(src, filepath.Join(binPath, name)))
}

// prepend our test bin directory to PATH
Expand Down Expand Up @@ -647,7 +682,8 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
})

// tweak mod time of elm formatter
as.NoError(test.RecreateSymlink(t, binPath+"/"+"elm-format"))
newTime := time.Now().Add(-time.Minute)
as.NoError(test.Lutimes(t, filepath.Join(binPath, "elm-format"), newTime, newTime))

_, statz, err = treefmt(t, args...)
as.NoError(err)
Expand All @@ -671,7 +707,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
})

// tweak mod time of python formatter
as.NoError(test.RecreateSymlink(t, binPath+"/"+"black"))
as.NoError(test.Lutimes(t, filepath.Join(binPath, "black"), newTime, newTime))

_, statz, err = treefmt(t, args...)
as.NoError(err)
Expand All @@ -695,11 +731,12 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
})

// add go formatter
cfg.FormatterConfigs["go"] = &config.Formatter{
goFormatter := &config.Formatter{
Command: "gofmt",
Options: []string{"-w"},
Includes: []string{"*.go"},
}
cfg.FormatterConfigs["go"] = goFormatter
test.WriteConfig(t, configPath, cfg)

_, statz, err = treefmt(t, args...)
Expand All @@ -723,6 +760,66 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
stats.Changed: 0,
})

// tweak go formatter options
goFormatter.Options = []string{"-w", "-s"}

test.WriteConfig(t, configPath, cfg)

_, statz, err = treefmt(t, args...)
as.NoError(err)

assertStats(t, as, statz, map[stats.Type]int{
stats.Traversed: 32,
stats.Matched: 4,
stats.Formatted: 4,
stats.Changed: 0,
})

// tweak go formatter includes
goFormatter.Includes = []string{"foo/*"}

test.WriteConfig(t, configPath, cfg)

_, statz, err = treefmt(t, args...)
as.NoError(err)

assertStats(t, as, statz, map[stats.Type]int{
stats.Traversed: 32,
stats.Matched: 3,
stats.Formatted: 3,
stats.Changed: 0,
})

// tweak go formatter excludes
goFormatter.Includes = []string{"*.go"}
goFormatter.Excludes = []string{"foo/*"}

test.WriteConfig(t, configPath, cfg)

_, statz, err = treefmt(t, args...)
as.NoError(err)

assertStats(t, as, statz, map[stats.Type]int{
stats.Traversed: 32,
stats.Matched: 4,
stats.Formatted: 4,
stats.Changed: 0,
})

// add a priority
cfg.FormatterConfigs["go"].Priority = 3
test.WriteConfig(t, configPath, cfg)

_, statz, err = treefmt(t, args...)
as.NoError(err)

assertStats(t, as, statz, map[stats.Type]int{
stats.Traversed: 32,
stats.Matched: 4,
stats.Formatted: 4,
stats.Changed: 0,
})

// remove python formatter
delete(cfg.FormatterConfigs, "python")
test.WriteConfig(t, configPath, cfg)
Expand Down Expand Up @@ -772,6 +869,35 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
stats.Formatted: 0,
stats.Changed: 0,
})

// change global excludes
cfg.Excludes = []string{"touch.toml"}
test.WriteConfig(t, configPath, cfg)

_, statz, err = treefmt(t, args...)
as.NoError(err)

assertStats(t, as, statz, map[stats.Type]int{
stats.Traversed: 32,
stats.Matched: 1,
stats.Formatted: 1,
stats.Changed: 0,
})

cfg.Excludes = nil
cfg.Global.Excludes = []string{"touch.toml", "yaml/test.yaml"}

test.WriteConfig(t, configPath, cfg)

_, statz, err = treefmt(t, args...)
as.NoError(err)

assertStats(t, as, statz, map[stats.Type]int{
stats.Traversed: 32,
stats.Matched: 1,
stats.Formatted: 1,
stats.Changed: 0,
})
}

func TestGit(t *testing.T) {
Expand Down
22 changes: 22 additions & 0 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,18 @@ import (
"path/filepath"
"strings"

"github.com/charmbracelet/log"
"github.com/numtide/treefmt/walk"
"github.com/spf13/pflag"
"github.com/spf13/viper"
)

var ErrInvalidBatchSize = fmt.Errorf("batch size must be between 1 and 10,240")

// Config is used to represent the list of configured Formatters.
type Config struct {
AllowMissingFormatter bool `mapstructure:"allow-missing-formatter" toml:"allow-missing-formatter,omitempty"`
BatchSize int `mapstructure:"batch-size" toml:"batch-size,omitempty"`
CI bool `mapstructure:"ci" toml:"ci,omitempty"`
ClearCache bool `mapstructure:"clear-cache" toml:"-"` // not allowed in config
CPUProfile string `mapstructure:"cpu-profile" toml:"cpu-profile,omitempty"`
Expand Down Expand Up @@ -59,6 +63,9 @@ func SetFlags(fs *pflag.FlagSet) {
"allow-missing-formatter", false,
"Do not exit with error if a configured formatter is missing. (env $TREEFMT_ALLOW_MISSING_FORMATTER)",
)
fs.Uint("batch-size", 1024,
"The maximum number of files to pass to a formatter at once. (env $TREEFMT_BATCH_SIZE)",
)
fs.Bool(
"ci", false,
"Runs treefmt in a CI mode, enabling --no-cache, --fail-on-change and adjusting some other settings "+
Expand Down Expand Up @@ -235,6 +242,21 @@ func FromViper(v *viper.Viper) (*Config, error) {
}
}

// validate batch size
// todo what is a reasonable upper limit on this?

// default if it isn't set (e.g. in tests when using Config directly)
if cfg.BatchSize == 0 {
cfg.BatchSize = 1024
}

if !(1 <= cfg.BatchSize && cfg.BatchSize <= 10240) {
return nil, ErrInvalidBatchSize
}

l := log.WithPrefix("config")
l.Infof("batch size = %d", cfg.BatchSize)

return cfg, nil
}

Expand Down
Loading