diff --git a/cmd/root_test.go b/cmd/root_test.go index ef5e493c..12dd95c7 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -802,210 +802,389 @@ func TestFailOnChange(t *testing.T) { }) } -func TestBustCacheOnFormatterChange(t *testing.T) { +func TestCacheBusting(t *testing.T) { as := require.New(t) - tempDir := test.TempExamples(t) - configPath := tempDir + "/touch.toml" + t.Run("formatter_change_config", func(t *testing.T) { + tempDir := test.TempExamples(t) + configPath := filepath.Join(tempDir, "treefmt.toml") - // symlink some formatters into temp dir, so we can mess with their mod times - binPath := filepath.Join(tempDir, "bin") - as.NoError(os.Mkdir(binPath, 0o755)) + test.ChangeWorkDir(t, tempDir) - binaries := []string{"black", "elm-format", "gofmt"} + // basic config + cfg := &config.Config{ + FormatterConfigs: map[string]*config.Formatter{ + "python": { + Command: "echo", // this is non-destructive, will match but cause no changes + Includes: []string{"*.py"}, + }, + "haskell": { + Command: "test-fmt-append", + Options: []string{" "}, + Includes: []string{"*.hs"}, + }, + }, + } - for _, name := range binaries { - src, err := exec.LookPath(name) - as.NoError(err) - as.NoError(os.Symlink(src, filepath.Join(binPath, name))) - } + // initial run + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 8, + stats.Formatted: 8, + stats.Changed: 6, + })) - // prepend our test bin directory to PATH - t.Setenv("PATH", binPath+":"+os.Getenv("PATH")) + // change formatter options + cfg.FormatterConfigs["haskell"].Options = []string{""} - // start with 2 formatters - cfg := &config.Config{ - FormatterConfigs: map[string]*config.Formatter{ - "python": { - Command: "black", - Includes: []string{"*.py"}, - }, - "elm": { - Command: "elm-format", - Options: []string{"--yes"}, - Includes: []string{"*.elm"}, - }, - }, - } + // cache entries for haskell files should be invalidated + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 8, + stats.Formatted: 6, + stats.Changed: 6, + })) - test.WriteConfig(t, configPath, cfg) - args := []string{"--config-file", configPath, "--tree-root", tempDir} - _, statz, err := treefmt(t, args...) - as.NoError(err) + // run again, nothing should be formatted + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 8, + stats.Formatted: 0, + stats.Changed: 0, + })) - assertStats(t, as, statz, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 3, - stats.Formatted: 3, - stats.Changed: 0, - }) + // change the formatter's command + cfg.FormatterConfigs["haskell"].Command = "echo" - // tweak mod time of elm formatter - newTime := time.Now().Add(-time.Minute) - as.NoError(test.Lutimes(t, filepath.Join(binPath, "elm-format"), newTime, newTime)) + // cache entries for haskell files should be invalidated + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 8, + stats.Formatted: 6, + stats.Changed: 0, // echo doesn't affect the files so no changes expected + })) - _, statz, err = treefmt(t, args...) - as.NoError(err) + // run again, nothing should be formatted + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 8, + stats.Formatted: 0, + stats.Changed: 0, + })) - assertStats(t, as, statz, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 3, - stats.Formatted: 1, - stats.Changed: 0, - }) + // change the formatters includes + cfg.FormatterConfigs["haskell"].Includes = []string{"haskell/*.hs"} - // check cache is working - _, statz, err = treefmt(t, args...) - as.NoError(err) + // we should match on fewer files, but no formatting should occur as includes are not part of the formatting + // signature + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 6, + stats.Formatted: 0, + stats.Changed: 0, + })) - assertStats(t, as, statz, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 3, - stats.Formatted: 0, - stats.Changed: 0, + // change the formatters excludes + cfg.FormatterConfigs["haskell"].Excludes = []string{"haskell/Foo.hs"} + + // we should match on fewer files, but no formatting should occur as excludes are not part of the formatting + // signature + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 5, + stats.Formatted: 0, + stats.Changed: 0, + })) }) - // tweak mod time of python formatter - as.NoError(test.Lutimes(t, filepath.Join(binPath, "black"), newTime, newTime)) + t.Run("formatter_change_binary", func(t *testing.T) { + tempDir := test.TempExamples(t) + configPath := filepath.Join(tempDir, "treefmt.toml") - _, statz, err = treefmt(t, args...) - as.NoError(err) + test.ChangeWorkDir(t, tempDir) - assertStats(t, as, statz, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 3, - stats.Formatted: 2, - stats.Changed: 0, - }) + // find test-fmt-append in PATH + sourcePath, err := exec.LookPath("test-fmt-append") + as.NoError(err, "failed to find test-fmt-append in PATH") - // check cache is working - _, statz, err = treefmt(t, args...) - as.NoError(err) + // copy it into the temp dir so we can mess with its size and modtime + binPath := filepath.Join(tempDir, "bin") + as.NoError(os.Mkdir(binPath, 0o755)) - assertStats(t, as, statz, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 3, - stats.Formatted: 0, - stats.Changed: 0, - }) + scriptPath := filepath.Join(binPath, "test-fmt-append") - // add go formatter - goFormatter := &config.Formatter{ - Command: "gofmt", - Options: []string{"-w"}, - Includes: []string{"*.go"}, - } - cfg.FormatterConfigs["go"] = goFormatter - test.WriteConfig(t, configPath, cfg) + test.CopyFile(t, sourcePath, scriptPath) + as.NoError(os.Chmod(scriptPath, 0o755)) - _, statz, err = treefmt(t, args...) - as.NoError(err) + // prepend our test bin directory to PATH + t.Setenv("PATH", binPath+":"+os.Getenv("PATH")) - assertStats(t, as, statz, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 4, - stats.Formatted: 1, - stats.Changed: 0, - }) + // basic config + cfg := &config.Config{ + FormatterConfigs: map[string]*config.Formatter{ + "python": { + Command: "echo", // this is non-destructive, will match but cause no changes + Includes: []string{"*.py"}, + }, + "elm": { + Command: "test-fmt-append", + Options: []string{" "}, + Includes: []string{"*.elm"}, + }, + }, + } - // check cache is working - _, statz, err = treefmt(t, args...) - as.NoError(err) + // initial run + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 33, + stats.Matched: 3, + stats.Formatted: 3, + stats.Changed: 1, + })) - assertStats(t, as, statz, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 4, - stats.Formatted: 0, - stats.Changed: 0, - }) + // tweak mod time of elm formatter + newTime := time.Now().Add(-time.Minute) + as.NoError(os.Chtimes(scriptPath, newTime, newTime)) - // tweak go formatter options - goFormatter.Options = []string{"-w", "-s"} + // cache entries for elm files should be invalidated + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 33, + stats.Matched: 3, + stats.Formatted: 1, + stats.Changed: 1, + })) - test.WriteConfig(t, configPath, cfg) + // running again with a hot cache, we should see nothing be formatted + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 33, + stats.Matched: 3, + stats.Formatted: 0, + stats.Changed: 0, + }), + ) - _, statz, err = treefmt(t, args...) - as.NoError(err) + // tweak the size of elm formatter + formatter, err := os.OpenFile(scriptPath, os.O_WRONLY|os.O_APPEND, 0o755) + as.NoError(err, "failed to open elm formatter") - assertStats(t, as, statz, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 4, - stats.Formatted: 1, - stats.Changed: 0, + _, err = formatter.Write([]byte(" ")) // add some whitespace + as.NoError(err, "failed to append to elm formatter") + as.NoError(formatter.Close(), "failed to close elm formatter") + + // cache entries for elm files should be invalidated + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 33, + stats.Matched: 3, + stats.Formatted: 1, + stats.Changed: 1, + })) + + // running again with a hot cache, we should see nothing be formatted + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 33, + stats.Matched: 3, + stats.Formatted: 0, + stats.Changed: 0, + }), + ) }) - // add a priority - cfg.FormatterConfigs["go"].Priority = 3 - test.WriteConfig(t, configPath, cfg) + t.Run("formatter_add_remove", func(t *testing.T) { + tempDir := test.TempExamples(t) + configPath := filepath.Join(tempDir, "treefmt.toml") - _, statz, err = treefmt(t, args...) - as.NoError(err) + test.ChangeWorkDir(t, tempDir) - assertStats(t, as, statz, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 4, - stats.Formatted: 1, - stats.Changed: 0, - }) + cfg := &config.Config{ + FormatterConfigs: map[string]*config.Formatter{ + "python": { + Command: "test-fmt-append", + Options: []string{" "}, + Includes: []string{"*.py"}, + }, + }, + } - // remove python formatter - delete(cfg.FormatterConfigs, "python") - test.WriteConfig(t, configPath, cfg) + // initial run + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 2, + stats.Formatted: 2, + stats.Changed: 2, + }), + ) - _, statz, err = treefmt(t, args...) - as.NoError(err) + // cached run + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 2, + stats.Formatted: 0, + stats.Changed: 0, + }), + ) - assertStats(t, as, statz, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 2, - stats.Formatted: 0, - stats.Changed: 0, - }) + // add a formatter + cfg.FormatterConfigs["elm"] = &config.Formatter{ + Command: "test-fmt-append", + Options: []string{" "}, + Includes: []string{"*.elm"}, + } - // check cache is working - _, statz, err = treefmt(t, args...) - as.NoError(err) + // only the elm files should be formatted + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 3, + stats.Formatted: 1, + stats.Changed: 1, + }), + ) - assertStats(t, as, statz, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 2, - stats.Formatted: 0, - stats.Changed: 0, - }) + // let's add a second python formatter + cfg.FormatterConfigs["python_secondary"] = &config.Formatter{ + Command: "test-fmt-append", + Options: []string{" "}, + Includes: []string{"*.py"}, + } - // remove elm formatter - delete(cfg.FormatterConfigs, "elm") - test.WriteConfig(t, configPath, cfg) + // python files should be formatted as their pipeline has changed + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 3, + stats.Formatted: 2, + stats.Changed: 2, + }), + ) - _, statz, err = treefmt(t, args...) - as.NoError(err) + // cached run + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 3, + stats.Formatted: 0, + stats.Changed: 0, + }), + ) - assertStats(t, as, statz, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 1, - stats.Formatted: 0, - stats.Changed: 0, - }) + // change ordering within a pipeline + cfg.FormatterConfigs["python"].Priority = 2 + cfg.FormatterConfigs["python_secondary"].Priority = 1 - // check cache is working - _, statz, err = treefmt(t, args...) - as.NoError(err) + // python files should be formatted as their pipeline has changed + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 3, + stats.Formatted: 2, + stats.Changed: 2, + }), + ) - assertStats(t, as, statz, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 1, - stats.Formatted: 0, - stats.Changed: 0, + // cached run + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 3, + stats.Formatted: 0, + stats.Changed: 0, + }), + ) + + // remove secondary python formatter + delete(cfg.FormatterConfigs, "python_secondary") + + // python files should be formatted as their pipeline has changed + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 3, + stats.Formatted: 2, + stats.Changed: 2, + }), + ) + + // cached run + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 3, + stats.Formatted: 0, + stats.Changed: 0, + }), + ) + + // remove the elm formatter + delete(cfg.FormatterConfigs, "elm") + + // only python files should match, but no formatting should occur as not formatting signatures have been + // affected + treefmt2(t, + withConfig(configPath, cfg), + withNoError(as), + withStats(as, map[stats.Type]int{ + stats.Traversed: 32, + stats.Matched: 2, + stats.Formatted: 0, + stats.Changed: 0, + }), + ) }) } diff --git a/format/formatter.go b/format/formatter.go index c207303c..e9d40d72 100644 --- a/format/formatter.go +++ b/format/formatter.go @@ -71,8 +71,7 @@ func (f *Formatter) Hash(h hash.Hash) error { // include the executable's size and mod time // if the formatter executable changes (e.g. new version) the outcome of applying the formatter might differ - h.Write([]byte(fmt.Sprintf("%d", info.Size()))) - h.Write([]byte(fmt.Sprintf("%d", info.ModTime().Unix()))) + h.Write([]byte(fmt.Sprintf("%d %d", info.Size(), info.ModTime().Unix()))) return nil } diff --git a/format/scheduler.go b/format/scheduler.go index 5920d5c0..490dd51b 100644 --- a/format/scheduler.go +++ b/format/scheduler.go @@ -83,6 +83,8 @@ func (s *scheduler) formattersSignature(key batchKey, formatters []*Formatter) ( // store the signature so we don't have to re-compute for each file s.signatures[key] = sig + log.Debugf("formatters signature for key %s: %x", key, sig) + return sig, nil } diff --git a/test/test.go b/test/test.go index b1d63205..fe4823d7 100644 --- a/test/test.go +++ b/test/test.go @@ -2,6 +2,7 @@ package test import ( "fmt" + "io" "os" "path/filepath" "testing" @@ -123,3 +124,23 @@ func ChangeWorkDir(t *testing.T, dir string) { t.Fatal(fmt.Errorf("failed to change working directory to %s: %w", dir, err)) } } + +func CopyFile(t *testing.T, sourcePath string, destPath string) { + t.Helper() + + source, err := os.Open(sourcePath) + if err != nil { + t.Fatal(err, fmt.Sprintf("failed to open %s", sourcePath)) + } + defer source.Close() + + dest, err := os.Create(destPath) + if err != nil { + t.Fatal(err, fmt.Sprintf("failed to create %s", sourcePath)) + } + defer dest.Close() + + if _, err = io.Copy(dest, source); err != nil { + t.Fatal(err, fmt.Sprintf("failed to copy %s to %s", sourcePath, destPath)) + } +}