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.

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

Add a command to clean all txtar output #143

Closed
sdboyer opened this issue May 10, 2023 · 4 comments · Fixed by #145
Closed

Add a command to clean all txtar output #143

sdboyer opened this issue May 10, 2023 · 4 comments · Fixed by #145
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@sdboyer
Copy link
Contributor

sdboyer commented May 10, 2023

Our txtar test framework leaves garbage behind. If a test that produces some output (anything out/*) goes away, the output still remains. I think everything on main is clean now, but with the already large and increasing volume of tests our txtar corpus produces, it's really hard to tell if there's garbage in there or not.

I mistakenly tried to fix this as part of the original txtar testing framework, internal/txtartest; it can error if there are outputs in the txtar that aren't produced by the test. But that makes it impossible to reuse the same txtar from multiple tests in different packages all across the project - exactly the use pattern we want to enable.

So instead, we should have a command we can call - i'd be fine with introducing something like mage - that can walk over all our txtar corpus(es) and remove all output. Once cleaned, running THEMA_UPDATE_GOLDEN=1 go test ./... should be sufficient to repopulate all txtar files correctly.

@sdboyer sdboyer added enhancement New feature or request help wanted Extra attention is needed labels May 10, 2023
@sdboyer sdboyer changed the title testing: Add a command to clean all txtar output Add a command to clean all txtar output May 10, 2023
@joanlopez
Copy link
Contributor

joanlopez commented May 10, 2023

You mean something like this magefile.go?

//go:build mage

package main

import (
	"golang.org/x/tools/txtar"
	"os"
	"path/filepath"
	"strings"
)

// Clean cleans all the output from corpus txtar files
func Clean() error {
	return filepath.Walk("./testdata", cleanTxtarOut)
}

func cleanTxtarOut(path string, _ os.FileInfo, err error) error {
	if err != nil {
		return err
	}

	if filepath.Ext(path) != ".txtar" {
		return nil
	}

	data, err := os.ReadFile(path)
	if err != nil {
		return err
	}

	archive := txtar.Parse(data)
	archive.Files = filter(
		archive.Files,
		func(t txtar.File) bool {
			return !strings.HasPrefix(t.Name, "out")
		},
	)

	return os.WriteFile(path, txtar.Format(archive), 0666)

}

func filter(ff []txtar.File, criteria func(f txtar.File) bool) []txtar.File {
	filtered := make([]txtar.File, 0, len(ff))
	for _, f := range ff {
		if criteria(f) {
			filtered = append(filtered, f)
		}
	}
	return filtered
}

@sdboyer
Copy link
Contributor Author

sdboyer commented May 11, 2023

that looks plausible to me - did you just have that ready, lol? or maybe chatgpt spat it out 😉

@joanlopez
Copy link
Contributor

joanlopez commented May 11, 2023

that looks plausible to me - did you just have that ready, lol? or maybe chatgpt spat it out 😉

Yeah, I already had it ready before you wrote the issue because I used it for #144 -- as I changed multiple times the path of the output files, I thought about a programmatic way to do such clean ups.

Honestly, I'm not going to negate I interacted with ChatGPT more than once at this point, but sincerely this time I felt like it was almost easier to write these lines than asking to it, plus the potential risk of having to waste time on fixing minor issues from its output. It's not the first time that I do similar things with txtar package, tho.

Indeed, the fun fact is that I initially wrote the filter function using generics, but when I moved it into the magefile.go, it started to complain, so instead of spending time on figuring out why, I decided to just replace it with a specific implementation hahaha 🙉

@joanlopez
Copy link
Contributor

So, do you want me to open a PR?

If so, should we think about somehow documenting/reflecting the use of Mage (use of Bingo or similar, and/or leaving a comment in the README), or would be fine to just leave the magefile.go on the root directory for now? 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants